fix: treat empty existing mount as stale in ensureMountPoint

pkg/secrets-store/nodeserver.go

@@ -71,7 +71,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	var providerName string
 	var podName, podNamespace, podUID string
 	var targetPath string
-	var mounted, isRemountRequest, skipped, isErrorMasked bool
+	var mounted, hasContent, isRemountRequest, skipped, isErrorMasked bool
 	errorReason := internalerrors.FailedToMount
 	rotationEnabled := ns.rotationConfig.enabled
 
@@ -137,7 +137,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 		}
 	}
 
-	mounted, err = ns.ensureMountPoint(targetPath)
+	mounted, hasContent, err = ns.ensureMountPoint(targetPath)
 	if err != nil {
 		// kubelet will not create the CSI NodePublishVolume target directory in 1.20+, in accordance with the CSI specification.
 		// CSI driver needs to properly create and process the target path
@@ -154,8 +154,8 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	isRemountRequest = mounted
 
 	// If rotation is not enabled, don't remount the already mounted secrets.
-	if !rotationEnabled && mounted {
-		klog.InfoS("target path is already mounted", "targetPath", targetPath, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName})
+	if !rotationEnabled && mounted && hasContent {
+		klog.InfoS("target path is already mounted and populated", "targetPath", targetPath, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName})
 		skipped = true
 		return &csi.NodePublishVolumeResponse{}, nil
 	}

pkg/secrets-store/utils.go

@@ -34,46 +34,43 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-// ensureMountPoint ensures mount point is valid
-func (ns *nodeServer) ensureMountPoint(target string) (bool, error) {
+// ensureMountPoint reports whether the target is mounted and whether
+// it has any content.
+func (ns *nodeServer) ensureMountPoint(target string) (mounted, hasContent bool, err error) {
 	notMnt, err := ns.mounter.IsLikelyNotMountPoint(target)
 	if err != nil {
-		return !notMnt, err
+		return false, false, err
 	}
 
 	if !notMnt {
-		// testing original mount point, make sure the mount link is valid
-		_, err := os.ReadDir(target)
-		if err == nil {
-			klog.InfoS("already mounted to target", "targetPath", target)
-			// already mounted
-			return !notMnt, nil
+		entries, readErr := os.ReadDir(target)
+		if readErr == nil {
+			klog.V(4).InfoS("already mounted to target", "targetPath", target)
+			return true, len(entries) > 0, nil
 		}
-		if err := ns.mounter.Unmount(target); err != nil {
-			klog.ErrorS(err, "failed to unmount directory", "targetPath", target)
-			return !notMnt, err
+		// Mount is in a bad state. Unmount and let the caller perform a
+		// fresh mount.
+		if unmountErr := ns.mounter.Unmount(target); unmountErr != nil {
+			klog.ErrorS(unmountErr, "failed to unmount directory", "targetPath", target)
+			return true, false, unmountErr
 		}
-		notMnt = true
-		// remount it in node publish
-		return !notMnt, err
+		return false, false, nil
 	}
 
 	if runtimeutil.IsRuntimeWindows() {
 		// IsLikelyNotMountPoint always returns notMnt=true for windows as the
 		// target path is not a soft link to the global mount
 		// instead check if the dir exists for windows and if it's not empty
 		// If there are contents in the dir, then objects are already mounted
-		f, err := os.ReadDir(target)
+		entries, err := os.ReadDir(target)
 		if err != nil {
-			return !notMnt, err
-		}
-		if len(f) > 0 {
-			notMnt = false
-			return !notMnt, err
+			return false, false, err
 		}
+		present := len(entries) > 0
+		return present, present, nil
 	}
 
-	return false, nil
+	return false, false, nil
 }
 
 func (ns *nodeServer) getLastUpdateTime(target string) (time.Time, error) {

pkg/secrets-store/utils_test.go

@@ -19,6 +19,8 @@ package secretsstore
 import (
 	"context"
 	"fmt"
+	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
 
@@ -28,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	mount "k8s.io/mount-utils"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
@@ -69,6 +72,82 @@ func newSecretProviderClassPodStatus(name, namespace, node string) *secretsstore
 	}
 }
 
+func TestEnsureMountPoint(t *testing.T) {
+	// newMountedFakeMounter returns a FakeMounter with target pre-registered
+	// as a tmpfs mount point, matching what nodeserver.go creates before the
+	// provider writes content.
+	newMountedFakeMounter := func(target string) *mount.FakeMounter {
+		return mount.NewFakeMounter([]mount.MountPoint{{Device: "tmpfs", Path: target, Type: "tmpfs"}})
+	}
+
+	tests := []struct {
+		name             string
+		setup            func(t *testing.T, target string) mount.Interface
+		wantMounted      bool
+		wantHasContent   bool
+		wantErr          bool
+		wantStillMounted bool
+	}{
+		{
+			name: "target is not mounted",
+			setup: func(t *testing.T, target string) mount.Interface {
+				return mount.NewFakeMounter([]mount.MountPoint{})
+			},
+		},
+		{
+			name: "target is mounted and populated",
+			setup: func(t *testing.T, target string) mount.Interface {
+				if err := os.WriteFile(filepath.Join(target, "secret1"), []byte("v"), 0600); err != nil {
+					t.Fatalf("seed: %v", err)
+				}
+				return newMountedFakeMounter(target)
+			},
+			wantMounted:      true,
+			wantHasContent:   true,
+			wantStillMounted: true,
+		},
+		{
+			// A previous NodePublishVolume was interrupted before writing
+			// files. The mount is reported as existing but empty; the caller
+			// re-calls the provider to populate it.
+			name: "target is mounted but empty",
+			setup: func(t *testing.T, target string) mount.Interface {
+				return newMountedFakeMounter(target)
+			},
+			wantMounted:      true,
+			wantHasContent:   false,
+			wantStillMounted: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			target := t.TempDir()
+			mounter := tt.setup(t, target)
+			ns := &nodeServer{mounter: mounter}
+
+			mounted, hasContent, err := ns.ensureMountPoint(target)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("ensureMountPoint err = %v, wantErr %v", err, tt.wantErr)
+			}
+			if mounted != tt.wantMounted {
+				t.Errorf("mounted = %v, want %v", mounted, tt.wantMounted)
+			}
+			if hasContent != tt.wantHasContent {
+				t.Errorf("hasContent = %v, want %v", hasContent, tt.wantHasContent)
+			}
+
+			notMnt, nmErr := mounter.IsLikelyNotMountPoint(target)
+			if nmErr != nil {
+				t.Fatalf("IsLikelyNotMountPoint after: %v", nmErr)
+			}
+			if stillMounted := !notMnt; stillMounted != tt.wantStillMounted {
+				t.Errorf("still mounted after = %v, want %v", stillMounted, tt.wantStillMounted)
+			}
+		})
+	}
+}
+
 func TestCreateOrUpdateSecretProviderClassPodStatus(t *testing.T) {
 	tests := []struct {
 		name   string