diff --git a/.github/workflows/trivy.yaml b/.github/workflows/trivy.yaml index 1df91e90bf0..4248fa20c32 100644 --- a/.github/workflows/trivy.yaml +++ b/.github/workflows/trivy.yaml @@ -12,7 +12,7 @@ jobs: - name: Set up Go 1.x uses: actions/setup-go@v5 with: - go-version: 1.22.5 + go-version: 1.23.1 id: go - name: Checkout code diff --git a/pkg/smb/controllerserver.go b/pkg/smb/controllerserver.go index e407eb23141..5f443084935 100644 --- a/pkg/smb/controllerserver.go +++ b/pkg/smb/controllerserver.go @@ -19,7 +19,6 @@ package smb import ( "context" "fmt" - "io/fs" "os" "os/exec" "path/filepath" @@ -92,12 +91,17 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) } volCap := volumeCapabilities[0] - if volCap.GetMount() != nil && !createSubDir { + if volCap.GetMount() != nil { options := volCap.GetMount().GetMountFlags() - if hasGuestMountOptions(options) { + if !createSubDir && hasGuestMountOptions(options) { klog.V(2).Infof("guest mount option(%v) is provided, create subdirectory", options) createSubDir = true } + // set default file/dir mode + volCap.GetMount().MountFlags = appendMountOptions(options, map[string]string{ + fileMode: defaultFileMode, + dirMode: defaultDirMode, + }) } if acquired := d.volumeLocks.TryAcquire(name); !acquired { @@ -159,18 +163,22 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) } defer d.volumeLocks.Release(volumeID) - var volCap *csi.VolumeCapability secrets := req.GetSecrets() mountOptions := getMountOptions(secrets) if mountOptions != "" { klog.V(2).Infof("DeleteVolume: found mountOptions(%v) for volume(%s)", mountOptions, volumeID) - volCap = &csi.VolumeCapability{ - AccessType: &csi.VolumeCapability_Mount{ - Mount: &csi.VolumeCapability_MountVolume{ - MountFlags: []string{mountOptions}, - }, + } + // set default file/dir mode + volCap := &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{ + MountFlags: appendMountOptions([]string{mountOptions}, + map[string]string{ + fileMode: defaultFileMode, + dirMode: defaultDirMode, + }), }, - } + }, } if smbVol.onDelete == "" { @@ -224,14 +232,6 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) return nil, status.Errorf(codes.Internal, "archive subdirectory(%s, %s) failed with %v", internalVolumePath, archivedInternalVolumePath, err) } } else { - if _, err2 := os.Lstat(internalVolumePath); err2 == nil { - err2 := filepath.WalkDir(internalVolumePath, func(path string, _ fs.DirEntry, _ error) error { - return os.Chmod(path, 0777) - }) - if err2 != nil { - klog.Errorf("failed to chmod subdirectory: %v", err2) - } - } klog.V(2).Infof("Removing subdirectory at %v", internalVolumePath) if err = os.RemoveAll(internalVolumePath); err != nil { return nil, status.Errorf(codes.Internal, "failed to delete subdirectory: %v", err) diff --git a/pkg/smb/smb.go b/pkg/smb/smb.go index e0a9034de92..d2d72c4e202 100644 --- a/pkg/smb/smb.go +++ b/pkg/smb/smb.go @@ -52,6 +52,10 @@ const ( DefaultKrb5CacheDirectory = "/var/lib/kubelet/kerberos/" retain = "retain" archive = "archive" + fileMode = "file_mode" + dirMode = "dir_mode" + defaultFileMode = "0777" + defaultDirMode = "0777" ) var supportedOnDeleteValues = []string{"", "delete", retain, archive} @@ -234,3 +238,28 @@ func validateOnDeleteValue(onDelete string) error { return fmt.Errorf("invalid value %s for OnDelete, supported values are %v", onDelete, supportedOnDeleteValues) } + +// appendMountOptions appends extra mount options to the given mount options +func appendMountOptions(mountOptions []string, extraMountOptions map[string]string) []string { + // stores the mount options already included in mountOptions + included := make(map[string]bool) + for _, mountOption := range mountOptions { + for k := range extraMountOptions { + if strings.HasPrefix(mountOption, k) { + included[k] = true + } + } + } + + allMountOptions := mountOptions + for k, v := range extraMountOptions { + if _, isIncluded := included[k]; !isIncluded { + if v != "" { + allMountOptions = append(allMountOptions, fmt.Sprintf("%s=%s", k, v)) + } else { + allMountOptions = append(allMountOptions, k) + } + } + } + return allMountOptions +} diff --git a/pkg/smb/smb_test.go b/pkg/smb/smb_test.go index 19fad4e352f..e1c82dba7d1 100644 --- a/pkg/smb/smb_test.go +++ b/pkg/smb/smb_test.go @@ -338,3 +338,50 @@ func TestValidateOnDeleteValue(t *testing.T) { } } } + +func TestAppendMountOptions(t *testing.T) { + tests := []struct { + desc string + options []string + newOpts map[string]string + expected []string + }{ + { + desc: "empty options", + options: nil, + newOpts: map[string]string{}, + expected: nil, + }, + { + desc: "empty newOpts", + options: []string{"a", "b"}, + newOpts: map[string]string{}, + expected: []string{"a", "b"}, + }, + { + desc: "empty newOpts", + options: []string{"a", "b"}, + newOpts: map[string]string{"c": "d"}, + expected: []string{"a", "b", "c=d"}, + }, + { + desc: "duplicate newOpts", + options: []string{"a", "b", "c=d"}, + newOpts: map[string]string{"c": "d"}, + expected: []string{"a", "b", "c=d"}, + }, + { + desc: "normal newOpts", + options: []string{"a", "b"}, + newOpts: map[string]string{"c": "d", "e": "f"}, + expected: []string{"a", "b", "c=d", "e=f"}, + }, + } + + for _, test := range tests { + result := appendMountOptions(test.options, test.newOpts) + if !reflect.DeepEqual(result, test.expected) { + t.Errorf("test[%s]: unexpected output: %v, expected result: %v", test.desc, result, test.expected) + } + } +}