Add check for content directory unchanged

The content directory is hashed at the beginning and then right before
switching. When there is any change detected, an event and log entry is
emitted. This is to minimize possible interactions with other
controllers or actors. The controller will retry and eventually replace
the directory anyway.

Author: tchap

pkg/operator/staticpod/certsyncpod/certsync_controller.go

@@ -1,7 +1,9 @@
 package certsyncpod
 
 import (
+	"bytes"
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -239,7 +241,6 @@ func (c *CertSyncController) sync(ctx context.Context, syncCtx factory.SyncConte
 
 		errors = append(errors, writeFiles(&realFS, c.eventRecorder, "secret", secret.ObjectMeta, contentDir, data, 0600))
 	}
-
 	return utilerrors.NewAggregate(errors)
 }
 
@@ -249,6 +250,7 @@ type fileSystem struct {
 	RemoveAll             func(path string) error
 	WriteFile             func(name string, data []byte, perm os.FileMode) error
 	SwapDirectoriesAtomic func(dirA, dirB string) error
+	HashDirectory         func(path string) ([]byte, error)
 }
 
 var realFS = fileSystem{
@@ -257,6 +259,7 @@ var realFS = fileSystem{
 	RemoveAll:             os.RemoveAll,
 	WriteFile:             os.WriteFile,
 	SwapDirectoriesAtomic: staticpod.SwapDirectoriesAtomic,
+	HashDirectory:         hashDirectory,
 }
 
 func writeFiles[C string | []byte](
@@ -276,14 +279,22 @@ func writeFiles[C string | []byte](
 	// This would all just increase complexity and require atomic swap on the OS level anyway.
 
 	// In case the target directory does not exist, create it so that the directory not existing is not a special case.
-	klog.Infof("Ensuring target directory %q exists ...", targetDir)
+	klog.Infof("Ensuring content directory %q exists ...", targetDir)
 	if err := fs.MkdirAll(targetDir, 0755); err != nil && !os.IsExist(err) {
 		eventRecorder.Warningf("CertificateUpdateFailed", "Failed creating content directory for %s: %s/%s: %v", typeName, o.Namespace, o.Name, err)
 		return err
 	}
 
+	// We make sure the target directory is unchanged while we prepare the switch by computing the directory hash.
+	klog.Infof("Hashing current content directory %q ...", targetDir)
+	targetDirHashBefore, err := fs.HashDirectory(targetDir)
+	if err != nil {
+		eventRecorder.Warningf("CertificateUpdateFailed", "Failed to hash current content directory for %s: %s/%s: %v", typeName, o.Namespace, o.Name, err)
+		return err
+	}
+
 	// Create a tmp source directory to be swapped.
-	klog.Infof("Ensuring source directory %q exists ...", targetDir)
+	klog.Infof("Creating temporary directory to swap for %q ...", targetDir)
 	tmpDir, err := fs.MkdirTemp(filepath.Dir(targetDir), filepath.Base(targetDir)+"-*")
 	if err != nil {
 		eventRecorder.Warningf("CertificateUpdateFailed", "Failed to create temporary directory for %s: %s/%s: %v", typeName, o.Namespace, o.Name, err)
@@ -306,6 +317,19 @@ func writeFiles[C string | []byte](
 		}
 	}
 
+	// Make sure the target directory hasn't changed in the meantime.
+	klog.Infof("Hashing current content directory %q again and ensuring it's unchanged ...", targetDir)
+	targetDirHashAfter, err := fs.HashDirectory(targetDir)
+	if err != nil {
+		eventRecorder.Warningf("CertificateUpdateFailed", "Failed to hash current content directory for %s: %s/%s: %v", typeName, o.Namespace, o.Name, err)
+		return err
+	}
+	if !bytes.Equal(targetDirHashBefore, targetDirHashAfter) {
+		eventRecorder.Warningf("CertificateUpdateFailed", "Content directory changed while preparing to apply an update for %s: %s/%s", typeName, o.Namespace, o.Name)
+		klog.Warningf("Content directory changed while preparing to apply an update: %q", targetDir)
+		return fmt.Errorf("content directory changed while preparing to apply an update: %q", targetDir)
+	}
+
 	// Swap directories atomically.
 	klog.Infof("Atomically swapping target directory %q with temporary directory %q for %s: %s/%s ...", targetDir, tmpDir, typeName, o.Namespace, o.Name)
 	if err := fs.SwapDirectoriesAtomic(targetDir, tmpDir); err != nil {

pkg/operator/staticpod/certsyncpod/certsync_controller_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -228,6 +229,21 @@ func TestWriteFiles(t *testing.T) {
 			}
 			return fs
 		}),
+		errorTestCase("directory unchanged on content directory hash mismatch", func() *fileSystem {
+			fs := newRealFS()
+			fs.HashDirectory = hashDirectoryFuncWithReturnValues("hash", "mismatch")
+			return fs
+		}),
+		errorTestCase("directory unchanged on content directory initial hash error", func() *fileSystem {
+			fs := newRealFS()
+			fs.HashDirectory = hashDirectoryFuncWithReturnValues("error: nuked")
+			return fs
+		}),
+		errorTestCase("directory unchanged on content directory subsequent hash error", func() *fileSystem {
+			fs := newRealFS()
+			fs.HashDirectory = hashDirectoryFuncWithReturnValues("hash", "error: nuked")
+			return fs
+		}),
 	}
 
 	for _, tc := range testCases {
@@ -271,3 +287,14 @@ func failToWriteNth(writeFile writeFileFunc, n int) writeFileFunc {
 		return writeFile(path, data, perm)
 	}
 }
+
+func hashDirectoryFuncWithReturnValues(retVals ...string) func(string) ([]byte, error) {
+	return func(path string) ([]byte, error) {
+		v := retVals[0]
+		retVals = retVals[1:]
+		if strings.HasPrefix(v, "error:") {
+			return nil, errors.New(strings.TrimPrefix(v, "error:"))
+		}
+		return []byte(v), nil
+	}
+}

pkg/operator/staticpod/certsyncpod/hash_directory.go

@@ -0,0 +1,30 @@
+package certsyncpod
+
+import (
+	"bytes"
+	"crypto/md5"
+	"encoding/binary"
+	"io/fs"
+	"path/filepath"
+)
+
+func hashDirectory(root string) ([]byte, error) {
+	var b bytes.Buffer
+	if err := filepath.Walk(root, func(path string, info fs.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		// We do not hash the root directory entry itself.
+		if path == root {
+			return nil
+		}
+
+		b.WriteString(path)
+		binary.Write(&b, binary.LittleEndian, info.ModTime().UnixNano())
+		return nil
+	}); err != nil {
+		return nil, err
+	}
+
+	return md5.New().Sum(b.Bytes()), nil
+}

pkg/operator/staticpod/certsyncpod/hash_directory_test.go

@@ -0,0 +1,56 @@
+package certsyncpod
+
+import (
+	"bytes"
+	"crypto/md5"
+	"encoding/binary"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestHashDirectory(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Create two files to be hashed.
+	pathA := filepath.Join(tmpDir, "1.txt")
+	if err := os.WriteFile(pathA, []byte("1"), 0644); err != nil {
+		t.Fatalf("Failed to write file %q: %v", pathA, err)
+	}
+
+	pathB := filepath.Join(tmpDir, "2.txt")
+	if err := os.WriteFile(pathB, []byte("2"), 0644); err != nil {
+		t.Fatalf("Failed to write file %q: %v", pathB, err)
+	}
+
+	// Get their mod times.
+	infoA, err := os.Lstat(pathA)
+	if err != nil {
+		t.Fatalf("Failed to stat file %q: %v", pathA, err)
+	}
+	infoB, err := os.Lstat(pathB)
+	if err != nil {
+		t.Fatalf("Failed to stat file %q: %v", pathB, err)
+	}
+
+	// Compute the expected hash.
+	var b bytes.Buffer
+	b.WriteString(pathA)
+	binary.Write(&b, binary.LittleEndian, infoA.ModTime().UnixNano())
+	b.WriteString(pathB)
+	binary.Write(&b, binary.LittleEndian, infoB.ModTime().UnixNano())
+	expectedHash := md5.New().Sum(b.Bytes())
+
+	// Compute the actual hash.
+	gotHash, err := hashDirectory(tmpDir)
+	if err != nil {
+		t.Fatalf("Failed to hash directory %q: %v", tmpDir, err)
+	}
+
+	// Compare both hashes.
+	if !bytes.Equal(gotHash, expectedHash) {
+		t.Errorf("Unexpected directory hash received:\n%v", cmp.Diff(expectedHash, gotHash))
+	}
+}