Pass FsGroup to NodeStageVolume

pkg/volume/csi/csi_attacher.go

@@ -374,7 +374,8 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo
 		accessMode,
 		nodeStageSecrets,
 		csiSource.VolumeAttributes,
-		mountOptions)
+		mountOptions,
+		deviceMounterArgs.FsGroup)
 
 	if err != nil {
 		return err

pkg/volume/csi/csi_attacher_test.go

@@ -1068,22 +1068,27 @@ func TestAttacherGetDeviceMountPath(t *testing.T) {
 }
 
 func TestAttacherMountDevice(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DelegateFSGroupToCSIDriver, true)()
+
 	pvName := "test-pv"
+	var testFSGroup int64 = 3000
 	nonFinalError := volumetypes.NewUncertainProgressError("")
 	transientError := volumetypes.NewTransientOperationFailure("")
 
 	testCases := []struct {
-		testName                string
+		testName                 string
-		volName                 string
+		volName                  string
-		devicePath              string
+		devicePath               string
-		deviceMountPath         string
+		deviceMountPath          string
-		stageUnstageSet         bool
+		stageUnstageSet          bool
-		shouldFail              bool
+		fsGroup                  *int64
-		createAttachment        bool
+		expectedVolumeMountGroup string
-		populateDeviceMountPath bool
+		shouldFail               bool
-		exitError               error
+		createAttachment         bool
-		spec                    *volume.Spec
+		populateDeviceMountPath  bool
-		watchTimeout            time.Duration
+		exitError                error
+		spec                     *volume.Spec
+		watchTimeout             time.Duration
 	}{
 		{
 			testName:         "normal PV",
@@ -1184,6 +1189,17 @@ func TestAttacherMountDevice(t *testing.T) {
 			shouldFail:              true,
 			spec:                    volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), true),
 		},
+		{
+			testName:                 "fsgroup set",
+			volName:                  "test-vol1",
+			devicePath:               "path1",
+			deviceMountPath:          "path2",
+			fsGroup:                  &testFSGroup,
+			expectedVolumeMountGroup: "3000",
+			stageUnstageSet:          true,
+			createAttachment:         true,
+			spec:                     volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false),
+		},
 	}
 
 	for _, tc := range testCases {
@@ -1209,7 +1225,7 @@ func TestAttacherMountDevice(t *testing.T) {
 				t.Fatalf("failed to create new attacher: %v", err0)
 			}
 			csiAttacher := getCsiAttacherFromVolumeAttacher(attacher, tc.watchTimeout)
-			csiAttacher.csiClient = setupClient(t, tc.stageUnstageSet)
+			csiAttacher.csiClient = setupClientWithVolumeMountGroup(t, tc.stageUnstageSet, true /* volumeMountGroupSet */)
 
 			if tc.deviceMountPath != "" {
 				tc.deviceMountPath = filepath.Join(tmpDir, tc.deviceMountPath)
@@ -1247,7 +1263,11 @@ func TestAttacherMountDevice(t *testing.T) {
 			}
 
 			// Run
-			err := csiAttacher.MountDevice(tc.spec, tc.devicePath, tc.deviceMountPath, volume.DeviceMounterArgs{})
+			err := csiAttacher.MountDevice(
+				tc.spec,
+				tc.devicePath,
+				tc.deviceMountPath,
+				volume.DeviceMounterArgs{FsGroup: tc.fsGroup})
 
 			// Verify
 			if err != nil {
@@ -1302,6 +1322,9 @@ func TestAttacherMountDevice(t *testing.T) {
 				if !reflect.DeepEqual(vol.MountFlags, tc.spec.PersistentVolume.Spec.MountOptions) {
 					t.Errorf("expected mount options: %v, got: %v", tc.spec.PersistentVolume.Spec.MountOptions, vol.MountFlags)
 				}
+				if vol.VolumeMountGroup != tc.expectedVolumeMountGroup {
+					t.Errorf("expected volume mount group %q, got: %q", tc.expectedVolumeMountGroup, vol.VolumeMountGroup)
+				}
 			}
 
 			// Verify the deviceMountPath was created by the plugin
@@ -1321,16 +1344,20 @@ func TestAttacherMountDevice(t *testing.T) {
 
 func TestAttacherMountDeviceWithInline(t *testing.T) {
 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)()
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DelegateFSGroupToCSIDriver, true)()
 	pvName := "test-pv"
+	var testFSGroup int64 = 3000
 	testCases := []struct {
-		testName        string
+		testName                 string
-		volName         string
+		volName                  string
-		devicePath      string
+		devicePath               string
-		deviceMountPath string
+		deviceMountPath          string
-		stageUnstageSet bool
+		fsGroup                  *int64
-		shouldFail      bool
+		expectedVolumeMountGroup string
-		spec            *volume.Spec
+		stageUnstageSet          bool
-		watchTimeout    time.Duration
+		shouldFail               bool
+		spec                     *volume.Spec
+		watchTimeout             time.Duration
 	}{
 		{
 			testName:        "normal PV",
@@ -1390,6 +1417,16 @@ func TestAttacherMountDeviceWithInline(t *testing.T) {
 			deviceMountPath: "path2",
 			shouldFail:      true,
 		},
+		{
+			testName:                 "fsgroup set",
+			volName:                  "test-vol1",
+			devicePath:               "path1",
+			deviceMountPath:          "path2",
+			fsGroup:                  &testFSGroup,
+			expectedVolumeMountGroup: "3000",
+			stageUnstageSet:          true,
+			spec:                     volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false),
+		},
 	}
 
 	for _, tc := range testCases {
@@ -1410,7 +1447,7 @@ func TestAttacherMountDeviceWithInline(t *testing.T) {
 				t.Fatalf("failed to create new attacher: %v", err0)
 			}
 			csiAttacher := getCsiAttacherFromVolumeAttacher(attacher, tc.watchTimeout)
-			csiAttacher.csiClient = setupClient(t, tc.stageUnstageSet)
+			csiAttacher.csiClient = setupClientWithVolumeMountGroup(t, tc.stageUnstageSet, true /* volumeMountGroup */)
 
 			if tc.deviceMountPath != "" {
 				tc.deviceMountPath = filepath.Join(tmpDir, tc.deviceMountPath)
@@ -1435,7 +1472,11 @@ func TestAttacherMountDeviceWithInline(t *testing.T) {
 			}()
 
 			// Run
-			err = csiAttacher.MountDevice(tc.spec, tc.devicePath, tc.deviceMountPath, volume.DeviceMounterArgs{})
+			err = csiAttacher.MountDevice(
+				tc.spec,
+				tc.devicePath,
+				tc.deviceMountPath,
+				volume.DeviceMounterArgs{FsGroup: tc.fsGroup})
 
 			// Verify
 			if err != nil {
@@ -1467,6 +1508,9 @@ func TestAttacherMountDeviceWithInline(t *testing.T) {
 				if vol.Path != tc.deviceMountPath {
 					t.Errorf("expected mount path: %s. got: %s", tc.deviceMountPath, vol.Path)
 				}
+				if vol.VolumeMountGroup != tc.expectedVolumeMountGroup {
+					t.Errorf("expected volume mount group %q, got: %q", tc.expectedVolumeMountGroup, vol.VolumeMountGroup)
+				}
 			}
 
 			wg.Wait()

pkg/volume/csi/csi_block.go

@@ -193,7 +193,8 @@ func (m *csiBlockMapper) stageVolumeForBlock(
 		accessMode,
 		nodeStageSecrets,
 		csiSource.VolumeAttributes,
-		nil /* MountOptions */)
+		nil, /* MountOptions */
+		nil /* fsGroup */)
 
 	if err != nil {
 		return "", err

pkg/volume/csi/csi_client.go

@@ -78,6 +78,7 @@ type csiClient interface {
 		secrets map[string]string,
 		volumeContext map[string]string,
 		mountOptions []string,
+		fsGroup *int64,
 	) error
 
 	NodeGetVolumeStats(
@@ -384,6 +385,7 @@ func (c *csiDriverClient) NodeStageVolume(ctx context.Context,
 	secrets map[string]string,
 	volumeContext map[string]string,
 	mountOptions []string,
+	fsGroup *int64,
 ) error {
 	klog.V(4).Info(log("calling NodeStageVolume rpc [volid=%s,staging_target_path=%s]", volID, stagingTargetPath))
 	if volID == "" {
@@ -425,11 +427,24 @@ func (c *csiDriverClient) NodeStageVolume(ctx context.Context,
 			Block: &csipbv1.VolumeCapability_BlockVolume{},
 		}
 	} else {
+		mountVolume := &csipbv1.VolumeCapability_MountVolume{
+			FsType:     fsType,
+			MountFlags: mountOptions,
+		}
+
+		if utilfeature.DefaultFeatureGate.Enabled(features.DelegateFSGroupToCSIDriver) {
+			supported, err := c.NodeSupportsVolumeMountGroup(ctx)
+			if err != nil {
+				return err
+			}
+
+			if supported && fsGroup != nil {
+				mountVolume.VolumeMountGroup = strconv.FormatInt(*fsGroup, 10 /* base */)
+			}
+		}
+
 		req.VolumeCapability.AccessType = &csipbv1.VolumeCapability_Mount{
-			Mount: &csipbv1.VolumeCapability_MountVolume{
+			Mount: mountVolume,
-				FsType:     fsType,
-				MountFlags: mountOptions,
-			},
 		}
 	}
 

pkg/volume/csi/csi_client_test.go

@@ -222,6 +222,7 @@ func (c *fakeCsiDriverClient) NodeStageVolume(ctx context.Context,
 	secrets map[string]string,
 	volumeContext map[string]string,
 	mountOptions []string,
+	fsGroup *int64,
 ) error {
 	c.t.Log("calling fake.NodeStageVolume...")
 	req := &csipbv1.NodeStageVolumeRequest{
@@ -241,11 +242,24 @@ func (c *fakeCsiDriverClient) NodeStageVolume(ctx context.Context,
 			Block: &csipbv1.VolumeCapability_BlockVolume{},
 		}
 	} else {
+		mountVolume := &csipbv1.VolumeCapability_MountVolume{
+			FsType:     fsType,
+			MountFlags: mountOptions,
+		}
+
+		if utilfeature.DefaultFeatureGate.Enabled(features.DelegateFSGroupToCSIDriver) {
+			supported, err := c.NodeSupportsVolumeMountGroup(ctx)
+			if err != nil {
+				return err
+			}
+
+			if supported && fsGroup != nil {
+				mountVolume.VolumeMountGroup = strconv.FormatInt(*fsGroup, 10 /* base */)
+			}
+		}
+
 		req.VolumeCapability.AccessType = &csipbv1.VolumeCapability_Mount{
-			Mount: &csipbv1.VolumeCapability_MountVolume{
+			Mount: mountVolume,
-				FsType:     fsType,
-				MountFlags: mountOptions,
-			},
 		}
 	}
 
@@ -572,6 +586,8 @@ func TestClientNodeUnpublishVolume(t *testing.T) {
 }
 
 func TestClientNodeStageVolume(t *testing.T) {
+	var testFSGroup int64 = 3000
+
 	tmpDir, err := utiltesting.MkTmpdir("csi-test")
 	if err != nil {
 		t.Fatalf("can't create temp dir: %v", err)
@@ -580,30 +596,78 @@ func TestClientNodeStageVolume(t *testing.T) {
 	testPath := filepath.Join(tmpDir, "/test/path")
 
 	testCases := []struct {
-		name              string
+		name                           string
-		volID             string
+		volID                          string
-		stagingTargetPath string
+		stagingTargetPath              string
-		fsType            string
+		fsType                         string
-		secrets           map[string]string
+		secrets                        map[string]string
-		mountOptions      []string
+		mountOptions                   []string
-		mustFail          bool
+		fsGroup                        *int64
-		err               error
+		delegateFSGroupFeatureGate     bool
+		driverSupportsVolumeMountGroup bool
+		expectedFSGroupInNodeStage     string
+		mustFail                       bool
+		err                            error
 	}{
 		{name: "test ok", volID: "vol-test", stagingTargetPath: testPath, fsType: "ext4", mountOptions: []string{"unvalidated"}},
 		{name: "missing volID", stagingTargetPath: testPath, mustFail: true},
 		{name: "missing target path", volID: "vol-test", mustFail: true},
 		{name: "bad fs", volID: "vol-test", stagingTargetPath: testPath, fsType: "badfs", mustFail: true},
 		{name: "grpc error", volID: "vol-test", stagingTargetPath: testPath, mustFail: true, err: errors.New("grpc error")},
+
+		{
+			name:                           "fsgroup provided, DelegateFSGroupToCSIDriver feature enabled, driver supports volume mount group; expect fsgroup to be passed to NodeStageVolume",
+			volID:                          "vol-test",
+			stagingTargetPath:              testPath,
+			fsType:                         "ext4",
+			fsGroup:                        &testFSGroup,
+			delegateFSGroupFeatureGate:     true,
+			driverSupportsVolumeMountGroup: true,
+			expectedFSGroupInNodeStage:     "3000",
+		},
+		{
+			name:                           "fsgroup not provided, DelegateFSGroupToCSIDriver feature enabled, driver supports volume mount group; expect fsgroup not to be passed to NodeStageVolume",
+			volID:                          "vol-test",
+			stagingTargetPath:              testPath,
+			fsType:                         "ext4",
+			fsGroup:                        nil,
+			delegateFSGroupFeatureGate:     true,
+			driverSupportsVolumeMountGroup: true,
+			expectedFSGroupInNodeStage:     "",
+		},
+		{
+			name:                           "fsgroup provided, DelegateFSGroupToCSIDriver feature enabled, driver does not support volume mount group; expect fsgroup not to be passed to NodeStageVolume",
+			volID:                          "vol-test",
+			stagingTargetPath:              testPath,
+			fsType:                         "ext4",
+			fsGroup:                        &testFSGroup,
+			delegateFSGroupFeatureGate:     true,
+			driverSupportsVolumeMountGroup: false,
+			expectedFSGroupInNodeStage:     "",
+		},
+		{
+			name:                           "fsgroup provided, DelegateFSGroupToCSIDriver feature disabled, driver supports volume mount group; expect fsgroup not to be passed to NodeStageVolume",
+			volID:                          "vol-test",
+			stagingTargetPath:              testPath,
+			fsType:                         "ext4",
+			fsGroup:                        &testFSGroup,
+			delegateFSGroupFeatureGate:     false,
+			driverSupportsVolumeMountGroup: true,
+			expectedFSGroupInNodeStage:     "",
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Logf("Running test case: %s", tc.name)
+
+		defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DelegateFSGroupToCSIDriver, tc.delegateFSGroupFeatureGate)()
+
+		nodeClient := fake.NewNodeClientWithVolumeMountGroup(true /* stagingCapable */, tc.driverSupportsVolumeMountGroup)
+		nodeClient.SetNextError(tc.err)
 		fakeCloser := fake.NewCloser(t)
 		client := &csiDriverClient{
 			driverName: "Fake Driver Name",
 			nodeV1ClientCreator: func(addr csiAddr, m *MetricsManager) (csipbv1.NodeClient, io.Closer, error) {
-				nodeClient := fake.NewNodeClient(false /* stagingCapable */)
-				nodeClient.SetNextError(tc.err)
 				return nodeClient, fakeCloser, nil
 			},
 		}
@@ -618,9 +682,15 @@ func TestClientNodeStageVolume(t *testing.T) {
 			tc.secrets,
 			map[string]string{"attr0": "val0"},
 			tc.mountOptions,
+			tc.fsGroup,
 		)
 		checkErr(t, tc.mustFail, err)
 
+		volumeMountGroup := nodeClient.GetNodeStagedVolumes()[tc.volID].VolumeMountGroup
+		if volumeMountGroup != tc.expectedFSGroupInNodeStage {
+			t.Errorf("expected VolumeMountGroup parameter in NodePublishVolumeRequest to be %q, got: %q", tc.expectedFSGroupInNodeStage, volumeMountGroup)
+		}
+
 		if !tc.mustFail {
 			fakeCloser.Check()
 		}

pkg/volume/csi/fake/fake_client.go

@@ -280,6 +280,7 @@ func (f *NodeClient) NodeStageVolume(ctx context.Context, req *csipb.NodeStageVo
 	if mounted != nil {
 		fsType = mounted.GetFsType()
 		csiVol.MountFlags = mounted.GetMountFlags()
+		csiVol.VolumeMountGroup = mounted.VolumeMountGroup
 	}
 	if !strings.Contains(fsTypes, fsType) {
 		return nil, errors.New("invalid fstype")