Skip to content

Commit dc4be68

Browse files
andyzhangxandyzhangx
committed
feat: add FSGroupChangePolicy param for NFS mount
1 parent 0dca1be commit dc4be68

File tree

10 files changed

+237
-7
lines changed

10 files changed

+237
-7
lines changed

‎docs/driver-parameters.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ nodeStageSecretRef.name | secret name that stores(check below examples):<br>`azu
9797
nodeStageSecretRef.namespace | secret namespace | k8s namespace | Yes |
9898
--- | **Following parameters are only for NFS protocol** | --- | --- |
9999
volumeAttributes.mountPermissions | mounted folder permissions | `0777` | No |
100+
fsGroupChangePolicy | indicates how volume's ownership will be changed by the driver, pod `securityContext.fsGroupChangePolicy` is ignored | `OnRootMismatch`(by default), `Always`, `None` | No | `OnRootMismatch`
100101
--- | **Following parameters are only for feature: blobfuse [Managed Identity and Service Principal Name auth](https://github.com/Azure/azure-storage-fuse/tree/blobfuse-1.4.5#environment-variables)** | --- | --- |
101102
volumeAttributes.AzureStorageAuthType | Authentication Type | `Key`, `SAS`, `MSI`, `SPN` | No | `Key`
102103
volumeAttributes.AzureStorageIdentityClientID | Identity Client ID | | No |

‎pkg/blob/blob.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ const (
114114
accessTierField = "accesstier"
115115
networkEndpointTypeField = "networkendpointtype"
116116
mountPermissionsField = "mountpermissions"
117+
fsGroupChangePolicyField = "fsgroupchangepolicy"
117118
useDataPlaneAPIField = "usedataplaneapi"
118119

119120
// See https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-containers--blobs--and-metadata#container-names
@@ -145,11 +146,14 @@ const (
145146
VolumeID = "volumeid"
146147

147148
defaultStorageEndPointSuffix = "core.windows.net"
149+
150+
FSGroupChangeNone = "None"
148151
)
149152

150153
var (
151-
supportedProtocolList = []string{Fuse, Fuse2, NFS}
152-
retriableErrors = []string{accountNotProvisioned, tooManyRequests, statusCodeNotFound, containerBeingDeletedDataplaneAPIError, containerBeingDeletedManagementAPIError, clientThrottled}
154+
supportedProtocolList = []string{Fuse, Fuse2, NFS}
155+
retriableErrors = []string{accountNotProvisioned, tooManyRequests, statusCodeNotFound, containerBeingDeletedDataplaneAPIError, containerBeingDeletedManagementAPIError, clientThrottled}
156+
supportedFSGroupChangePolicyList = []string{FSGroupChangeNone, string(v1.FSGroupChangeAlways), string(v1.FSGroupChangeOnRootMismatch)}
153157
)
154158

155159
// DriverOptions defines driver parameters specified in driver deployment
@@ -169,6 +173,7 @@ type DriverOptions struct {
169173
VolStatsCacheExpireInMinutes int
170174
SasTokenExpirationMinutes int
171175
EnableVolumeMountGroup bool
176+
FSGroupChangePolicy string
172177
}
173178

174179
func (option *DriverOptions) AddFlags() {
@@ -187,6 +192,7 @@ func (option *DriverOptions) AddFlags() {
187192
flag.IntVar(&option.VolStatsCacheExpireInMinutes, "vol-stats-cache-expire-in-minutes", 10, "The cache expire time in minutes for volume stats cache")
188193
flag.IntVar(&option.SasTokenExpirationMinutes, "sas-token-expiration-minutes", 1440, "sas token expiration minutes during volume cloning")
189194
flag.BoolVar(&option.EnableVolumeMountGroup, "enable-volume-mount-group", true, "indicates whether enabling VOLUME_MOUNT_GROUP")
195+
flag.StringVar(&option.FSGroupChangePolicy, "fsgroup-change-policy", "", "indicates how the volume's ownership will be changed by the driver, OnRootMismatch is the default value")
190196
}
191197

192198
// Driver implements all interfaces of CSI drivers
@@ -207,6 +213,7 @@ type Driver struct {
207213
mountPermissions uint64
208214
enableAznfsMount bool
209215
enableVolumeMountGroup bool
216+
fsGroupChangePolicy string
210217
mounter *mount.SafeFormatAndMount
211218
volLockMap *util.LockMap
212219
// A map storing all volumes with ongoing operations so that additional operations
@@ -231,7 +238,6 @@ type Driver struct {
231238
// NewDriver Creates a NewCSIDriver object. Assumes vendor version is equal to driver version &
232239
// does not support optional driver plugin info manifest field. Refer to CSI spec for more details.
233240
func NewDriver(options *DriverOptions, kubeClient kubernetes.Interface, cloud *provider.Cloud) *Driver {
234-
var err error
235241
d := Driver{
236242
volLockMap: util.NewLockMap(),
237243
subnetLockMap: util.NewLockMap(),
@@ -247,6 +253,7 @@ func NewDriver(options *DriverOptions, kubeClient kubernetes.Interface, cloud *p
247253
mountPermissions: options.MountPermissions,
248254
enableAznfsMount: options.EnableAznfsMount,
249255
sasTokenExpirationMinutes: options.SasTokenExpirationMinutes,
256+
fsGroupChangePolicy: options.FSGroupChangePolicy,
250257
azcopy: &util.Azcopy{},
251258
KubeClient: kubeClient,
252259
cloud: cloud,
@@ -255,6 +262,7 @@ func NewDriver(options *DriverOptions, kubeClient kubernetes.Interface, cloud *p
255262
d.Version = driverVersion
256263
d.NodeID = options.NodeID
257264

265+
var err error
258266
getter := func(key string) (interface{}, error) { return nil, nil }
259267
if d.accountSearchCache, err = azcache.NewTimedCache(time.Minute, getter, false); err != nil {
260268
klog.Fatalf("%v", err)
@@ -1024,3 +1032,15 @@ func replaceWithMap(str string, m map[string]string) string {
10241032
}
10251033
return str
10261034
}
1035+
1036+
func isSupportedFSGroupChangePolicy(policy string) bool {
1037+
if policy == "" {
1038+
return true
1039+
}
1040+
for _, v := range supportedFSGroupChangePolicyList {
1041+
if policy == v {
1042+
return true
1043+
}
1044+
}
1045+
return false
1046+
}

‎pkg/blob/blob_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1783,3 +1783,42 @@ func TestDriverOptions_AddFlags(t *testing.T) {
17831783
}
17841784
})
17851785
}
1786+
1787+
func TestIsSupportedFSGroupChangePolicy(t *testing.T) {
1788+
tests := []struct {
1789+
policy string
1790+
expectedResult bool
1791+
}{
1792+
{
1793+
policy: "",
1794+
expectedResult: true,
1795+
},
1796+
{
1797+
policy: "None",
1798+
expectedResult: true,
1799+
},
1800+
{
1801+
policy: "Always",
1802+
expectedResult: true,
1803+
},
1804+
{
1805+
policy: "OnRootMismatch",
1806+
expectedResult: true,
1807+
},
1808+
{
1809+
policy: "onRootMismatch",
1810+
expectedResult: false,
1811+
},
1812+
{
1813+
policy: "invalid",
1814+
expectedResult: false,
1815+
},
1816+
}
1817+
1818+
for _, test := range tests {
1819+
result := isSupportedFSGroupChangePolicy(test.policy)
1820+
if result != test.expectedResult {
1821+
t.Errorf("isSupportedFSGroupChangePolicy(%s) returned with %v, not equal to %v", test.policy, result, test.expectedResult)
1822+
}
1823+
}
1824+
}

‎pkg/blob/controllerserver.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
9999
}
100100
var storageAccountType, subsID, resourceGroup, location, account, containerName, containerNamePrefix, protocol, customTags, secretName, secretNamespace, pvcNamespace string
101101
var isHnsEnabled, requireInfraEncryption, enableBlobVersioning, createPrivateEndpoint, enableNfsV3 *bool
102-
var vnetResourceGroup, vnetName, subnetName, accessTier, networkEndpointType, storageEndpointSuffix string
102+
var vnetResourceGroup, vnetName, subnetName, accessTier, networkEndpointType, storageEndpointSuffix, fsGroupChangePolicy string
103103
var matchTags, useDataPlaneAPI, getLatestAccountKey bool
104104
var softDeleteBlobs, softDeleteContainers int32
105105
var vnetResourceIDs []string
@@ -212,6 +212,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
212212
}
213213
case useDataPlaneAPIField:
214214
useDataPlaneAPI = strings.EqualFold(v, trueValue)
215+
case fsGroupChangePolicyField:
216+
fsGroupChangePolicy = v
215217
default:
216218
return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid parameter %q in storage class", k))
217219
}
@@ -223,6 +225,10 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
223225
}
224226
}
225227

228+
if !isSupportedFSGroupChangePolicy(fsGroupChangePolicy) {
229+
return nil, status.Errorf(codes.InvalidArgument, "fsGroupChangePolicy(%s) is not supported, supported fsGroupChangePolicy list: %v", fsGroupChangePolicy, supportedFSGroupChangePolicyList)
230+
}
231+
226232
if matchTags && account != "" {
227233
return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("matchTags must set as false when storageAccount(%s) is provided", account))
228234
}

‎pkg/blob/controllerserver_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,29 @@ func TestCreateVolume(t *testing.T) {
238238
}
239239
},
240240
},
241+
{
242+
name: "Invalid fsGroupChangePolicy",
243+
testFunc: func(t *testing.T) {
244+
d := NewFakeDriver()
245+
d.cloud = &azure.Cloud{}
246+
mp := map[string]string{
247+
fsGroupChangePolicyField: "test_fsGroupChangePolicy",
248+
}
249+
req := &csi.CreateVolumeRequest{
250+
Name: "unit-test",
251+
VolumeCapabilities: stdVolumeCapabilities,
252+
Parameters: mp,
253+
}
254+
d.Cap = []*csi.ControllerServiceCapability{
255+
controllerServiceCapability,
256+
}
257+
_, err := d.CreateVolume(context.Background(), req)
258+
expectedErr := status.Errorf(codes.InvalidArgument, "fsGroupChangePolicy(test_fsGroupChangePolicy) is not supported, supported fsGroupChangePolicy list: [None Always OnRootMismatch]")
259+
if !reflect.DeepEqual(err, expectedErr) {
260+
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
261+
}
262+
},
263+
},
241264
{
242265
name: "invalid getLatestAccountKey value",
243266
testFunc: func(t *testing.T) {

‎pkg/blob/nodeserver.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,8 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
252252

253253
containerNameReplaceMap := map[string]string{}
254254

255+
fsGroupChangePolicy := d.fsGroupChangePolicy
256+
255257
mountPermissions := d.mountPermissions
256258
performChmodOp := (mountPermissions > 0)
257259
for k, v := range attrib {
@@ -287,9 +289,15 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
287289
mountPermissions = perm
288290
}
289291
}
292+
case fsGroupChangePolicyField:
293+
fsGroupChangePolicy = v
290294
}
291295
}
292296

297+
if !isSupportedFSGroupChangePolicy(fsGroupChangePolicy) {
298+
return nil, status.Errorf(codes.InvalidArgument, "fsGroupChangePolicy(%s) is not supported, supported fsGroupChangePolicy list: %v", fsGroupChangePolicy, supportedFSGroupChangePolicyList)
299+
}
300+
293301
mnt, err := d.ensureMountPoint(targetPath, fs.FileMode(mountPermissions))
294302
if err != nil {
295303
return nil, status.Errorf(codes.Internal, "Could not mount target %q: %v", targetPath, err)
@@ -349,6 +357,13 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
349357
klog.V(2).Infof("skip chmod on targetPath(%s) since mountPermissions is set as 0", targetPath)
350358
}
351359

360+
if volumeMountGroup != "" && fsGroupChangePolicy != FSGroupChangeNone {
361+
klog.V(2).Infof("set gid of volume(%s) as %s using fsGroupChangePolicy(%s)", volumeID, volumeMountGroup, fsGroupChangePolicy)
362+
if err := volumehelper.SetVolumeOwnership(targetPath, volumeMountGroup, fsGroupChangePolicy); err != nil {
363+
return nil, status.Error(codes.Internal, fmt.Sprintf("SetVolumeOwnership with volume(%s) on %s failed with %v", volumeID, targetPath, err))
364+
}
365+
}
366+
352367
isOperationSucceeded = true
353368
klog.V(2).Infof("volume(%s) mount %s on %s succeeded", volumeID, source, targetPath)
354369
return &csi.NodeStageVolumeResponse{}, nil

‎pkg/blob/nodeserver_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,25 @@ func TestNodeStageVolume(t *testing.T) {
463463
}
464464
},
465465
},
466+
{
467+
name: "[Error] Invalid fsGroupChangePolicy",
468+
testFunc: func(t *testing.T) {
469+
req := &csi.NodeStageVolumeRequest{
470+
VolumeId: "unit-test",
471+
StagingTargetPath: "unit-test",
472+
VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap},
473+
VolumeContext: map[string]string{
474+
fsGroupChangePolicyField: "test_fsGroupChangePolicy",
475+
},
476+
}
477+
d := NewFakeDriver()
478+
_, err := d.NodeStageVolume(context.TODO(), req)
479+
expectedErr := status.Error(codes.InvalidArgument, "fsGroupChangePolicy(test_fsGroupChangePolicy) is not supported, supported fsGroupChangePolicy list: [None Always OnRootMismatch]")
480+
if !reflect.DeepEqual(err, expectedErr) {
481+
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
482+
}
483+
},
484+
},
466485
{
467486
name: "[Error] Could not mount to target",
468487
testFunc: func(t *testing.T) {

‎pkg/util/util.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,18 @@ import (
2121
"os"
2222
"os/exec"
2323
"regexp"
24+
"strconv"
2425
"strings"
2526
"sync"
2627

2728
"github.com/go-ini/ini"
2829
"github.com/pkg/errors"
30+
v1 "k8s.io/api/core/v1"
2931
"k8s.io/client-go/kubernetes"
3032
"k8s.io/client-go/rest"
3133
"k8s.io/client-go/tools/clientcmd"
3234
"k8s.io/klog/v2"
35+
"k8s.io/kubernetes/pkg/volume"
3336
)
3437

3538
const (
@@ -341,3 +344,46 @@ func GetKubeClient(kubeconfig string, kubeAPIQPS float64, kubeAPIBurst int, user
341344
kubeCfg.UserAgent = userAgent
342345
return kubernetes.NewForConfig(kubeCfg)
343346
}
347+
348+
type VolumeMounter struct {
349+
path string
350+
attributes volume.Attributes
351+
}
352+
353+
func (l *VolumeMounter) GetPath() string {
354+
return l.path
355+
}
356+
357+
func (l *VolumeMounter) GetAttributes() volume.Attributes {
358+
return l.attributes
359+
}
360+
361+
func (l *VolumeMounter) CanMount() error {
362+
return nil
363+
}
364+
365+
func (l *VolumeMounter) SetUp(_ volume.MounterArgs) error {
366+
return nil
367+
}
368+
369+
func (l *VolumeMounter) SetUpAt(_ string, _ volume.MounterArgs) error {
370+
return nil
371+
}
372+
373+
func (l *VolumeMounter) GetMetrics() (*volume.Metrics, error) {
374+
return nil, nil
375+
}
376+
377+
// SetVolumeOwnership would set gid for path recursively
378+
func SetVolumeOwnership(path, gid, policy string) error {
379+
id, err := strconv.Atoi(gid)
380+
if err != nil {
381+
return fmt.Errorf("convert %s to int failed with %v", gid, err)
382+
}
383+
gidInt64 := int64(id)
384+
fsGroupChangePolicy := v1.FSGroupChangeOnRootMismatch
385+
if policy != "" {
386+
fsGroupChangePolicy = v1.PodFSGroupChangePolicy(policy)
387+
}
388+
return volume.SetVolumeOwnership(&VolumeMounter{path: path}, path, &gidInt64, &fsGroupChangePolicy, nil)
389+
}

‎pkg/util/util_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,3 +600,59 @@ users:
600600
}
601601
}
602602
}
603+
604+
func TestSetVolumeOwnership(t *testing.T) {
605+
tmpVDir, err := os.MkdirTemp(os.TempDir(), "SetVolumeOwnership")
606+
if err != nil {
607+
t.Fatalf("can't make a temp dir: %v", err)
608+
}
609+
//deferred clean up
610+
defer os.RemoveAll(tmpVDir)
611+
612+
tests := []struct {
613+
path string
614+
gid string
615+
fsGroupChangePolicy string
616+
expectedError error
617+
}{
618+
{
619+
path: "path",
620+
gid: "",
621+
expectedError: fmt.Errorf("convert %s to int failed with %v", "", `strconv.Atoi: parsing "": invalid syntax`),
622+
},
623+
{
624+
path: "path",
625+
gid: "alpha",
626+
expectedError: fmt.Errorf("convert %s to int failed with %v", "alpha", `strconv.Atoi: parsing "alpha": invalid syntax`),
627+
},
628+
{
629+
path: "not-exists",
630+
gid: "1000",
631+
expectedError: fmt.Errorf("lstat not-exists: no such file or directory"),
632+
},
633+
{
634+
path: tmpVDir,
635+
gid: "1000",
636+
expectedError: nil,
637+
},
638+
{
639+
path: tmpVDir,
640+
gid: "1000",
641+
fsGroupChangePolicy: "Always",
642+
expectedError: nil,
643+
},
644+
{
645+
path: tmpVDir,
646+
gid: "1000",
647+
fsGroupChangePolicy: "OnRootMismatch",
648+
expectedError: nil,
649+
},
650+
}
651+
652+
for _, test := range tests {
653+
err := SetVolumeOwnership(test.path, test.gid, test.fsGroupChangePolicy)
654+
if err != nil && (err.Error() != test.expectedError.Error()) {
655+
t.Errorf("unexpected error: %v, expected error: %v", err, test.expectedError)
656+
}
657+
}
658+
}

0 commit comments

Comments
 (0)