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,92 @@
+//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 os.FileMode
+		expectChmod   bool
+	}{
+		{
+			desc:          "setgid bit 02770",
+			initialMode:   0770,
+			requestedMode: 02770,
+			expectChmod:   true,
+		},
+		{
+			desc:          "sticky bit 01777",
+			initialMode:   0777,
+			requestedMode: 01777,
+			expectChmod:   true,
+		},
+		{
+			desc:          "setgid already set 02770 -> 02770 no change",
+			initialMode:   02770,
+			requestedMode: 02770,
+			expectChmod:   false,
+		},
+		{
+			desc:          "setuid bit 04755",
+			initialMode:   0755,
+			requestedMode: 04755,
+			expectChmod:   true,
+		},
+	}
+
+	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
+			expectedMode := uint32(test.requestedMode)
+			if actualMode != expectedMode {
+				t.Errorf("expected mode 0%o, got 0%o", expectedMode, 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/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,44 @@ func getMountOptions(context map[string]string) string {
 	return ""
 }
 
+// fileModeToUnixMode converts a raw Unix mode value (stored as os.FileMode)
+// to its proper os.FileMode representation with correct Go bit positions for
+// setuid, setgid, and sticky bits.
+func fileModeToUnixMode(mode os.FileMode) os.FileMode {
+	goMode := 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 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.
+// The mode parameter is a raw Unix mode value (e.g. 02770) stored as os.FileMode.
+// Compares both regular permission bits (0777) and special bits (setuid/setgid/sticky)
+// to avoid unnecessary chmod calls while still detecting special-bit differences.
 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 := syscall.Chmod(targetPath, uint32(mode)); err != nil {
+	// Convert the raw Unix mode to Go's FileMode representation for comparison.
+	desiredMode := fileModeToUnixMode(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, currentMode, desiredMode)
+		if err := chmod(targetPath, uint32(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, currentMode)
 	}
 	return nil
 }