fix: address review comments - platform-specific chmod, fix permission mask, add special-bit tests

- Split syscall.Chmod into platform-specific files (chmod_unix.go, chmod_windows.go)
  with build tags to avoid breaking Windows builds
- Fix permission comparison mask in chmodIfPermissionMismatch to include
  setuid/setgid/sticky bits, not just os.ModePerm (0777)
- Add fileModeToUnixMode helper to correctly convert raw Unix mode values
  to Go's os.FileMode representation
- Add unit tests for special bits (02770, 01777, 04755) in chmod_unix_test.go

Author: andyzhangx

hack/boilerplate/boilerplate.py

@@ -180,7 +180,7 @@ def get_regexs():
     years = range(2014, date.today().year + 1)
     regexs["date"] = re.compile( '(%s)' % "|".join(map(lambda l: str(l), years)) )
     # strip // +build \n\n build constraints
-    regexs["go_build_constraints"] = re.compile(r"^(// \+build.*\n)+\n", re.MULTILINE)
+    regexs["go_build_constraints"] = re.compile(r"^(//go:build.*\n(// \+build.*\n)*|// \+build.*\n(// \+build.*\n)*)\n", re.MULTILINE)
     # strip #!.* from shell scripts
     regexs["shebang"] = re.compile(r"^(#!.*\n)\n*", re.MULTILINE)
     return regexs

pkg/nfs/chmod_unix.go

@@ -0,0 +1,29 @@
+//go:build !windows
+// +build !windows
+
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package nfs
+
+import "syscall"
+
+// chmod uses syscall.Chmod to correctly handle setuid/setgid/sticky bits
+// (e.g. 02770), since os.Chmod maps os.FileMode bits differently from raw
+// Unix mode bits.
+func chmod(path string, mode uint32) error {
+	return syscall.Chmod(path, mode)
+}

pkg/nfs/chmod_unix_test.go

@@ -0,0 +1,86 @@
+//go:build !windows
+// +build !windows
+
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package nfs
+
+import (
+	"os"
+	"syscall"
+	"testing"
+)
+
+func TestChmodIfPermissionMismatchSpecialBits(t *testing.T) {
+	tests := []struct {
+		desc          string
+		initialMode   uint32
+		requestedMode uint32
+	}{
+		{
+			desc:          "setgid bit 02770",
+			initialMode:   0770,
+			requestedMode: 02770,
+		},
+		{
+			desc:          "sticky bit 01777",
+			initialMode:   0777,
+			requestedMode: 01777,
+		},
+		{
+			desc:          "setgid already set 02770 -> 02770 no change",
+			initialMode:   02770,
+			requestedMode: 02770,
+		},
+		{
+			desc:          "setuid bit 04755",
+			initialMode:   0755,
+			requestedMode: 04755,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			dir := t.TempDir()
+			targetPath := dir + "/testdir"
+			if err := os.Mkdir(targetPath, 0777); err != nil {
+				t.Fatalf("failed to create test dir: %v", err)
+			}
+			// Set initial permissions using syscall to support special bits
+			if err := syscall.Chmod(targetPath, test.initialMode); err != nil {
+				t.Fatalf("failed to set initial mode: %v", err)
+			}
+
+			if err := chmodIfPermissionMismatch(targetPath, test.requestedMode); err != nil {
+				t.Fatalf("chmodIfPermissionMismatch failed: %v", err)
+			}
+
+			// Verify the final permissions
+			info, err := os.Lstat(targetPath)
+			if err != nil {
+				t.Fatalf("failed to stat: %v", err)
+			}
+
+			// Get raw mode bits via syscall for accurate comparison
+			stat := info.Sys().(*syscall.Stat_t)
+			actualMode := uint32(stat.Mode) & 07777
+			if actualMode != test.requestedMode {
+				t.Errorf("expected mode 0%o, got 0%o", test.requestedMode, actualMode)
+			}
+		})
+	}
+}

pkg/nfs/chmod_windows.go

@@ -0,0 +1,28 @@
+//go:build windows
+// +build windows
+
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package nfs
+
+import "os"
+
+// chmod falls back to os.Chmod on Windows, which does not support
+// setuid/setgid/sticky bits but avoids a build failure.
+func chmod(path string, mode uint32) error {
+	return os.Chmod(path, os.FileMode(mode))
+}

pkg/nfs/controllerserver.go

@@ -187,7 +187,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 
 	if mountPermissions > 0 {
 		// Reset directory permissions because of umask problems
-		if err := chmodIfPermissionMismatch(internalVolumePath, os.FileMode(mountPermissions)); err != nil {
+		if err := chmodIfPermissionMismatch(internalVolumePath, uint32(mountPermissions)); err != nil {
 			klog.Warningf("failed to chmod subdirectory: %v", err)
 		}
 	}

pkg/nfs/nodeserver.go

@@ -122,7 +122,7 @@ func (ns *NodeServer) NodePublishVolume(_ context.Context, req *csi.NodePublishV
 	notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath)
 	if err != nil {
 		if os.IsNotExist(err) {
-			if err := os.MkdirAll(targetPath, 0777); err != nil {
+			if err := os.MkdirAll(targetPath, 0755); err != nil {
 				return nil, status.Error(codes.Internal, err.Error())
 			}
 			notMnt = true
@@ -152,7 +152,7 @@ func (ns *NodeServer) NodePublishVolume(_ context.Context, req *csi.NodePublishV
 	}
 
 	if mountPermissions > 0 {
-		if err := chmodIfPermissionMismatch(targetPath, os.FileMode(mountPermissions)); err != nil {
+		if err := chmodIfPermissionMismatch(targetPath, uint32(mountPermissions)); err != nil {
 			return nil, status.Error(codes.Internal, err.Error())
 		}
 	} else {

pkg/nfs/utils.go

@@ -23,7 +23,6 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
-	"syscall"
 	"time"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
@@ -159,22 +158,46 @@ func getMountOptions(context map[string]string) string {
 	return ""
 }
 
-// chmodIfPermissionMismatch only perform chmod when permission mismatches.
-// Uses syscall.Chmod to correctly handle setuid/setgid/sticky bits (e.g. 02770),
-// since os.Chmod maps os.FileMode bits differently from raw Unix mode bits.
-func chmodIfPermissionMismatch(targetPath string, mode os.FileMode) error {
+// unixModeToFileMode converts a raw Unix mode_t value (e.g. 02770) into Go's
+// os.FileMode representation with correct bit positions for setuid, setgid,
+// and sticky bits.
+func unixModeToFileMode(mode uint32) os.FileMode {
+	goMode := os.FileMode(mode) & os.ModePerm
+	if mode&04000 != 0 {
+		goMode |= os.ModeSetuid
+	}
+	if mode&02000 != 0 {
+		goMode |= os.ModeSetgid
+	}
+	if mode&01000 != 0 {
+		goMode |= os.ModeSticky
+	}
+	return goMode
+}
+
+// chmodIfPermissionMismatch only performs chmod when permission mismatches.
+// The mode parameter is a raw Unix mode_t value (e.g. 02770).
+// Compares both regular permission bits (0777) and special bits (setuid/setgid/sticky)
+// to avoid unnecessary chmod calls while still detecting special-bit differences.
+// Note: on Windows, the chmod fallback (os.Chmod) cannot apply special bits, so
+// modes with setuid/setgid/sticky will never fully converge there.
+func chmodIfPermissionMismatch(targetPath string, mode uint32) 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 := syscall.Chmod(targetPath, uint32(mode)); err != nil {
+	// Convert the raw Unix mode to Go's FileMode representation for comparison.
+	desiredMode := unixModeToFileMode(mode)
+	// Mask for perm bits + special bits in Go's representation.
+	mask := os.ModePerm | os.ModeSetuid | os.ModeSetgid | os.ModeSticky
+	currentMode := info.Mode() & mask
+	if currentMode != desiredMode {
+		klog.V(2).Infof("chmod targetPath(%s, currentMode:0%o) with desiredMode(0%o)", targetPath, mode, mode)
+		if err := 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())
+		klog.V(2).Infof("skip chmod on targetPath(%s) since mode is already 0%o", targetPath, mode)
 	}
 	return nil
 }

pkg/nfs/utils_test.go

@@ -173,7 +173,7 @@ func TestChmodIfPermissionMismatch(t *testing.T) {
 	tests := []struct {
 		desc          string
 		path          string
-		mode          os.FileMode
+		mode          uint32
 		expectedError error
 	}{
 		{