Validate fstype on CreateVolume, but not NodePublishVolume

There are existing PVs that could have wrong fstype, this change tries to workaround this to validate fstype during CreateVolume for new volumes, not on NodePublishVolume
kubernetes-sigs · Jun 7, 2023 · 47ed8e9 · 47ed8e9
1 parent de1b767
commit 47ed8e9
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 23 deletions.
diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go
@@ -76,6 +76,9 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 	if err := d.isValidVolumeCapabilities(volCaps); err != nil {
 		return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Volume capabilities not supported: %s", err))
 	}
+	if err := d.validateFStype(volCaps); err != nil {
+		return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Volume fstype not supported: %s", err))
+	}
 
 	var (
 		azName           string

diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go
@@ -522,6 +522,54 @@ func TestCreateVolume(t *testing.T) {
 				mockCtl.Finish()
 			},
 		},
+		{
+			name: "Fail: Volume fsType not supported",
+			testFunc: func(t *testing.T) {
+				mockCtl := gomock.NewController(t)
+				mockCloud := mocks.NewMockCloud(mockCtl)
+
+				driver := &Driver{
+					endpoint:     endpoint,
+					cloud:        mockCloud,
+					gidAllocator: NewGidAllocator(),
+					tags:         parseTagsFromStr(""),
+				}
+
+				req := &csi.CreateVolumeRequest{
+					Name: volumeName,
+					VolumeCapabilities: []*csi.VolumeCapability{
+						{
+							AccessType: &csi.VolumeCapability_Mount{
+								Mount: &csi.VolumeCapability_MountVolume{
+									FsType: "abc",
+								},
+							},
+							AccessMode: &csi.VolumeCapability_AccessMode{
+								Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER,
+							},
+						},
+					},
+					CapacityRange: &csi.CapacityRange{
+						RequiredBytes: capacityRange,
+					},
+					Parameters: map[string]string{
+						ProvisioningMode: "efs-ap",
+						FsId:             fsId,
+						GidMin:           "1000",
+						GidMax:           "2000",
+						DirectoryPerms:   "777",
+						AzName:           "us-east-1a",
+					},
+				}
+
+				ctx := context.Background()
+				_, err := driver.CreateVolume(ctx, req)
+				if err == nil {
+					t.Fatal("CreateVolume did not fail")
+				}
+				mockCtl.Finish()
+			},
+		},
 		{
 			name: "Fail: AccessType is block",
 			testFunc: func(t *testing.T) {

diff --git a/pkg/driver/node.go b/pkg/driver/node.go
@@ -319,9 +319,6 @@ func (d *Driver) isValidVolumeCapabilities(volCaps []*csi.VolumeCapability) erro
 		return err
 	}
 
-	if err := d.validateFStype(volCaps); err != nil {
-		return err
-	}
 	return nil
 }
 

diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go
@@ -435,26 +435,6 @@ func TestNodePublishVolume(t *testing.T) {
 				message: "Volume capability not supported: only filesystem volumes are supported",
 			},
 		},
-		{
-			name: "fail: unsupported volume fstype capability",
-			req: &csi.NodePublishVolumeRequest{
-				VolumeId: volumeId,
-				VolumeCapability: &csi.VolumeCapability{
-					AccessType: &csi.VolumeCapability_Mount{
-						Mount: &csi.VolumeCapability_MountVolume{FsType: "abc"},
-					},
-					AccessMode: &csi.VolumeCapability_AccessMode{
-						Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER,
-					},
-				},
-				TargetPath: targetPath,
-			},
-			expectMakeDir: false,
-			expectError: errtyp{
-				code:    "InvalidArgument",
-				message: "Volume capability not supported: invalid fstype: abc",
-			},
-		},
 		{
 			name: "fail: multiple unsupported volume capabilities",
 			req: &csi.NodePublishVolumeRequest{