From 463a7a799d2e6bd43c8199f08a12c97e4a74f685 Mon Sep 17 00:00:00 2001 From: Justin Alvarez Date: Wed, 7 Jan 2026 23:51:06 +0000 Subject: [PATCH 1/3] Don't try to chown/chmod on Windows Signed-off-by: Justin Alvarez --- ecs-agent/daemonimages/csidriver/driver/node.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ecs-agent/daemonimages/csidriver/driver/node.go b/ecs-agent/daemonimages/csidriver/driver/node.go index 79f9dee6ecd..485c6140956 100644 --- a/ecs-agent/daemonimages/csidriver/driver/node.go +++ b/ecs-agent/daemonimages/csidriver/driver/node.go @@ -23,6 +23,7 @@ import ( "context" "fmt" "os" + "runtime" "strconv" "strings" @@ -259,13 +260,16 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol sourceVolumeHostPath = strings.TrimPrefix(target, EBSPathPrefix) } - // Gid is generated based on SourceVolumeHostPath - gid := util.GenerateGIDFromPath(sourceVolumeHostPath) - // Set permissions on the mount point to allow non-root users to access it - if err := setMountPointPermissions(target, gid); err != nil { - return nil, status.Errorf(codes.Internal, "Failed to set permissions on mount point %s: %v", target, err) + // chown/chmod don't work on Windows + if runtime.GOOS != "windows" { + // Gid is generated based on SourceVolumeHostPath + gid := util.GenerateGIDFromPath(sourceVolumeHostPath) + // Set permissions on the mount point to allow non-root users to access it + if err := setMountPointPermissions(target, gid); err != nil { + return nil, status.Errorf(codes.Internal, "Failed to set permissions on mount point %s: %v", target, err) + } + klog.V(4).InfoS("Successfully set permissions on mount point", "target", target, "volumeID", volumeID, "gid", gid) } - klog.V(4).InfoS("Successfully set permissions on mount point", "target", target, "volumeID", volumeID, "gid", gid) return &csi.NodeStageVolumeResponse{}, nil } From 3775ac6eaa585a6047c14601772cfe7e79e71190 Mon Sep 17 00:00:00 2001 From: Justin Alvarez Date: Thu, 8 Jan 2026 22:39:04 +0000 Subject: [PATCH 2/3] remove use of runtime platform Signed-off-by: Justin Alvarez --- .../daemonimages/csidriver/driver/node.go | 31 ++++--------------- .../csidriver/driver/node_linux.go | 16 ++++++++++ .../csidriver/driver/node_windows.go | 7 +++++ 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/ecs-agent/daemonimages/csidriver/driver/node.go b/ecs-agent/daemonimages/csidriver/driver/node.go index 485c6140956..b2edd3efaea 100644 --- a/ecs-agent/daemonimages/csidriver/driver/node.go +++ b/ecs-agent/daemonimages/csidriver/driver/node.go @@ -23,7 +23,6 @@ import ( "context" "fmt" "os" - "runtime" "strconv" "strings" @@ -260,33 +259,15 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol sourceVolumeHostPath = strings.TrimPrefix(target, EBSPathPrefix) } - // chown/chmod don't work on Windows - if runtime.GOOS != "windows" { - // Gid is generated based on SourceVolumeHostPath - gid := util.GenerateGIDFromPath(sourceVolumeHostPath) - // Set permissions on the mount point to allow non-root users to access it - if err := setMountPointPermissions(target, gid); err != nil { - return nil, status.Errorf(codes.Internal, "Failed to set permissions on mount point %s: %v", target, err) - } - klog.V(4).InfoS("Successfully set permissions on mount point", "target", target, "volumeID", volumeID, "gid", gid) - } - - return &csi.NodeStageVolumeResponse{}, nil -} - -// setMountPointPermissions sets the permissions on the mount point to allow non-root users to access it -func setMountPointPermissions(mountPath string, gid int) error { - // Change group ownership to the provided GID - if err := chownFunc(mountPath, -1, gid); err != nil { - return fmt.Errorf("failed to change group ownership of %s to GID %d: %v", mountPath, gid, err) - } + // Gid is generated based on SourceVolumeHostPath + gid := util.GenerateGIDFromPath(sourceVolumeHostPath) - // Set permissions to 0775 with setgid bit - if err := chmodFunc(mountPath, 0775|os.ModeSetgid); err != nil { - return fmt.Errorf("failed to set permissions on %s: %v", mountPath, err) + // Set permissions on the mount point to allow non-root users to access it + if err := setMountPointPermissions(target, gid, volumeID); err != nil { + return nil, status.Errorf(codes.Internal, "Failed to set permissions on mount point %s: %v", target, err) } - return nil + return &csi.NodeStageVolumeResponse{}, nil } func newNodeService() nodeService { diff --git a/ecs-agent/daemonimages/csidriver/driver/node_linux.go b/ecs-agent/daemonimages/csidriver/driver/node_linux.go index 73b94abb5e0..f59906b639a 100644 --- a/ecs-agent/daemonimages/csidriver/driver/node_linux.go +++ b/ecs-agent/daemonimages/csidriver/driver/node_linux.go @@ -182,3 +182,19 @@ func (d *nodeService) getBlockSizeBytes(devicePath string, _ string) (int64, err } return gotSizeBytes, nil } + +// setMountPointPermissions sets the permissions on the mount point to allow non-root users to access it +func setMountPointPermissions(mountPath string, gid int, volumeID string) error { + // Change group ownership to the provided GID + if err := chownFunc(mountPath, -1, gid); err != nil { + return fmt.Errorf("failed to change group ownership of %s to GID %d: %v", mountPath, gid, err) + } + + // Set permissions to 0775 with setgid bit + if err := chmodFunc(mountPath, 0775|os.ModeSetgid); err != nil { + return fmt.Errorf("failed to set permissions on %s: %v", mountPath, err) + } + + klog.V(4).InfoS("Successfully set permissions on mount point", "target", mountPath, "volumeID", volumeID, "gid", gid) + return nil +} diff --git a/ecs-agent/daemonimages/csidriver/driver/node_windows.go b/ecs-agent/daemonimages/csidriver/driver/node_windows.go index 970a4605ae8..818074a286b 100644 --- a/ecs-agent/daemonimages/csidriver/driver/node_windows.go +++ b/ecs-agent/daemonimages/csidriver/driver/node_windows.go @@ -28,6 +28,7 @@ import ( "strings" "github.com/aws/amazon-ecs-agent/ecs-agent/daemonimages/csidriver/mounter" + "k8s.io/klog/v2" ) // getBlockSizeBytes gets the size of the disk in bytes @@ -75,3 +76,9 @@ func (d *nodeService) findDevicePath(devicePath, volumeID, _ string) (string, er return foundDiskNumber, nil } + +// setMountPointPermissions is a no-op on Windows because chown/chmod don't work on Windows +func setMountPointPermissions(_ string, _ int, _ string) error { + klog.V(4).InfoS("Skipping setting mount point permissions on Windows") + return nil +} From d34df8b08ce88a643e3938ab5ef57a4611e3298d Mon Sep 17 00:00:00 2001 From: Justin Alvarez Date: Thu, 8 Jan 2026 22:50:35 +0000 Subject: [PATCH 3/3] fix test Signed-off-by: Justin Alvarez --- ecs-agent/daemonimages/csidriver/driver/node_linux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ecs-agent/daemonimages/csidriver/driver/node_linux_test.go b/ecs-agent/daemonimages/csidriver/driver/node_linux_test.go index 7bb76f7a7ec..13aa17e08ec 100644 --- a/ecs-agent/daemonimages/csidriver/driver/node_linux_test.go +++ b/ecs-agent/daemonimages/csidriver/driver/node_linux_test.go @@ -991,7 +991,7 @@ func TestSetMountPointPermissions(t *testing.T) { } // Call the function - err := setMountPointPermissions(tc.mountPath, tc.gid) + err := setMountPointPermissions(tc.mountPath, tc.gid, "volumeID") // Verify results if tc.expectedError {