From 47ed8e981fca71b1c0c5b445110674a3a1d99059 Mon Sep 17 00:00:00 2001 From: jinxu Date: Wed, 7 Jun 2023 11:28:30 -0700 Subject: [PATCH] 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 --- pkg/driver/controller.go | 3 +++ pkg/driver/controller_test.go | 48 +++++++++++++++++++++++++++++++++++ pkg/driver/node.go | 3 --- pkg/driver/node_test.go | 20 --------------- 4 files changed, 51 insertions(+), 23 deletions(-) diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index aaf920e65..9e41cacfc 100644 --- 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 index 96f4790a9..151757b97 100644 --- 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 index cd73422f5..11fdd7600 100644 --- 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 index 20c55cd90..3ef2f2daa 100644 --- 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{