Merge pull request #676 from andyzhangx/bypass-chmod

fix: bypass chmod if mounting point permissions are correct

Author: andyzhangx

pkg/blob/blob.go

@@ -18,6 +18,7 @@ package blob
 
 import (
 	"fmt"
+	"os"
 	"strings"
 	"sync"
 	"time"
@@ -747,3 +748,21 @@ func appendDefaultMountOptions(mountOptions []string, tmpPath, containerName str
 
 	return allMountOptions
 }
+
+// chmodIfPermissionMismatch only perform chmod when permission mismatches
+func chmodIfPermissionMismatch(targetPath string, mode os.FileMode) error {
+	info, err := os.Lstat(targetPath)
+	if err != nil {
+		return err
+	}
+	perm := info.Mode() & os.ModePerm
+	if perm != mode {
+		klog.V(2).Infof("chmod targetPath(%s, mode:0%o) with permissions(0%o)", targetPath, info.Mode(), mode)
+		if err := os.Chmod(targetPath, mode); err != nil {
+			return err
+		}
+	} else {
+		klog.V(2).Infof("skip chmod on targetPath(%s) since mode is already 0%o)", targetPath, info.Mode())
+	}
+	return nil
+}

pkg/blob/blob_test.go

@@ -889,3 +889,57 @@ func TestIsSupportedContainerNamePrefix(t *testing.T) {
 		}
 	}
 }
+
+func TestChmodIfPermissionMismatch(t *testing.T) {
+	permissionMatchingPath, _ := getWorkDirPath("permissionMatchingPath")
+	_ = makeDir(permissionMatchingPath)
+	defer os.RemoveAll(permissionMatchingPath)
+
+	permissionMismatchPath, _ := getWorkDirPath("permissionMismatchPath")
+	_ = os.MkdirAll(permissionMismatchPath, os.FileMode(0721))
+	defer os.RemoveAll(permissionMismatchPath)
+
+	tests := []struct {
+		desc          string
+		path          string
+		mode          os.FileMode
+		expectedError error
+	}{
+		{
+			desc:          "Invalid path",
+			path:          "invalid-path",
+			mode:          0755,
+			expectedError: fmt.Errorf("CreateFile invalid-path: The system cannot find the file specified"),
+		},
+		{
+			desc:          "permission matching path",
+			path:          permissionMatchingPath,
+			mode:          0755,
+			expectedError: nil,
+		},
+		{
+			desc:          "permission mismatch path",
+			path:          permissionMismatchPath,
+			mode:          0755,
+			expectedError: nil,
+		},
+	}
+
+	for _, test := range tests {
+		err := chmodIfPermissionMismatch(test.path, test.mode)
+		if !reflect.DeepEqual(err, test.expectedError) {
+			if err == nil || test.expectedError == nil && !strings.Contains(err.Error(), test.expectedError.Error()) {
+				t.Errorf("test[%s]: unexpected error: %v, expected error: %v", test.desc, err, test.expectedError)
+			}
+		}
+	}
+}
+
+// getWorkDirPath returns the path to the current working directory
+func getWorkDirPath(dir string) (string, error) {
+	path, err := os.Getwd()
+	if err != nil {
+		return "", err
+	}
+	return fmt.Sprintf("%s%c%s", path, os.PathSeparator, dir), nil
+}

pkg/blob/nodeserver.go

@@ -291,10 +291,8 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 		}
 
 		if performChmodOp {
-			klog.V(2).Infof("volumeID(%v): chmod targetPath(%s) with permissions(0%o)", volumeID, targetPath, mountPermissions)
-			// set permissions for NFSv3 root folder
-			if err := os.Chmod(targetPath, os.FileMode(mountPermissions)); err != nil {
-				return nil, status.Error(codes.Internal, fmt.Sprintf("Chmod(%s) failed with %v", targetPath, err))
+			if err := chmodIfPermissionMismatch(targetPath, os.FileMode(mountPermissions)); err != nil {
+				return nil, status.Error(codes.Internal, err.Error())
 			}
 		} else {
 			klog.V(2).Infof("skip chmod on targetPath(%s) since mountPermissions is set as 0", targetPath)