GIDs are not allocated correctly above certain volume threshold

pkg/cloud/cloud.go

@@ -38,6 +38,7 @@ const (
 	AccessDeniedException    = "AccessDeniedException"
 	AccessPointAlreadyExists = "AccessPointAlreadyExists"
 	PvcNameTagKey            = "pvcName"
+	AccessPointPerFsLimit    = 1000
 )
 
 var (
@@ -266,7 +267,7 @@ func (c *cloud) findAccessPointByClientToken(ctx context.Context, clientToken st
 	klog.V(2).Infof("ClientToken to find AP : %s", clientToken)
 	describeAPInput := &efs.DescribeAccessPointsInput{
 		FileSystemId: &accessPointOpts.FileSystemId,
-		MaxResults:   aws.Int64(1000),
+		MaxResults:   aws.Int64(AccessPointPerFsLimit),
 	}
 	res, err := c.efs.DescribeAccessPointsWithContext(ctx, describeAPInput)
 	if err != nil {
@@ -296,6 +297,7 @@ func (c *cloud) findAccessPointByClientToken(ctx context.Context, clientToken st
 func (c *cloud) ListAccessPoints(ctx context.Context, fileSystemId string) (accessPoints []*AccessPoint, err error) {
 	describeAPInput := &efs.DescribeAccessPointsInput{
 		FileSystemId: &fileSystemId,
+		MaxResults:   aws.Int64(AccessPointPerFsLimit),
 	}
 	res, err := c.efs.DescribeAccessPointsWithContext(ctx, describeAPInput)
 	if err != nil {

pkg/driver/controller_test.go

@@ -215,8 +215,8 @@ func TestCreateVolume(t *testing.T) {
 						AccessPointId: apId,
 						FileSystemId:  fsId,
 						PosixUser: &cloud.PosixUser{
-							Gid: 1003,
-							Uid: 1003,
+							Gid: 1001,
+							Uid: 1001,
 						},
 					},
 					{
@@ -229,7 +229,7 @@ func TestCreateVolume(t *testing.T) {
 					},
 				}
 
-				var expectedGid int64 = 1001 //1003 and 1002 are taken, next available is 1001
+				var expectedGid int64 = 1003 //1001 and 1002 are taken, next available is 1003
 				mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil)
 				mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(accessPoints, nil)
 				mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), false).Return(accessPoint, nil).
@@ -429,7 +429,7 @@ func TestCreateVolume(t *testing.T) {
 				}
 
 				accessPoints := []*cloud.AccessPoint{}
-				for i := 0; i < ACCESS_POINT_PER_FS_LIMIT; i++ {
+				for i := 0; i < cloud.AccessPointPerFsLimit; i++ {
 					gidMin, err := strconv.Atoi(req.Parameters[GidMin])
 					if err != nil {
 						t.Fatalf("Failed to convert GidMax Parameter to int.")
@@ -488,6 +488,69 @@ func TestCreateVolume(t *testing.T) {
 				mockCtl.Finish()
 			},
 		},
+		{
+			name: "Success: GID range exceeds EFS access point limit",
+			testFunc: func(t *testing.T) {
+				mockCtl := gomock.NewController(t)
+				mockCloud := mocks.NewMockCloud(mockCtl)
+
+				driver := &Driver{
+					endpoint:     endpoint,
+					cloud:        mockCloud,
+					gidAllocator: NewGidAllocator(),
+				}
+
+				req := &csi.CreateVolumeRequest{
+					Name: volumeName,
+					VolumeCapabilities: []*csi.VolumeCapability{
+						stdVolCap,
+					},
+					CapacityRange: &csi.CapacityRange{
+						RequiredBytes: capacityRange,
+					},
+					Parameters: map[string]string{
+						ProvisioningMode: "efs-ap",
+						FsId:             fsId,
+						DirectoryPerms:   "777",
+						BasePath:         "test",
+						GidMin:           "1000",
+						GidMax:           "1000000",
+					},
+				}
+
+				ctx := context.Background()
+				fileSystem := &cloud.FileSystem{
+					FileSystemId: fsId,
+				}
+
+				accessPoint := &cloud.AccessPoint{
+					AccessPointId: apId,
+					FileSystemId:  fsId,
+				}
+
+				expectedGid := 1000 // Allocator should pick lowest available GID
+				mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil)
+				mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(nil, nil)
+				mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), false).Return(accessPoint, nil).
+					Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions, reuseAccessPointName bool) {
+						if accessPointOpts.Uid != int64(expectedGid) {
+							t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid)
+						}
+						if accessPointOpts.Gid != int64(expectedGid) {
+							t.Fatalf("Gid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Gid)
+						}
+					})
+
+				var err error
+
+				_, err = driver.CreateVolume(ctx, req)
+				if err != nil {
+					t.Fatalf("CreateVolume failed unexpectedly: %v", err)
+				}
+
+				mockCtl.Finish()
+			},
+		},
 		{
 			name: "Success: Normal flow",
 			testFunc: func(t *testing.T) {

pkg/driver/gid_allocator.go

@@ -12,8 +12,6 @@ import (
 	"k8s.io/klog/v2"
 )
 
-var ACCESS_POINT_PER_FS_LIMIT int = 1000
-
 type FilesystemID struct {
 	gidMin int
 	gidMax int
@@ -85,9 +83,10 @@ func (g *GidAllocator) getUsedGids(ctx context.Context, localCloud cloud.Cloud,
 func getNextUnusedGid(usedGids []int64, gidMin, gidMax int) (nextGid int, err error) {
 	requestedRange := gidMax - gidMin
 
-	if requestedRange > ACCESS_POINT_PER_FS_LIMIT {
-		klog.Warningf("Requested GID range (%v:%v) exceeds EFS Access Point limit (%v) per Filesystem. Driver will not allocate GIDs outside of this limit.", gidMin, gidMax, ACCESS_POINT_PER_FS_LIMIT)
-		gidMin = gidMax - ACCESS_POINT_PER_FS_LIMIT
+	if requestedRange > cloud.AccessPointPerFsLimit {
+		overrideGidMax := gidMin + cloud.AccessPointPerFsLimit
+		klog.Warningf("Requested GID range (%v:%v) exceeds EFS Access Point limit (%v) per Filesystem. Driver will use limited GID range (%v:%v)", gidMin, gidMax, cloud.AccessPointPerFsLimit, gidMin, overrideGidMax)
+		gidMax = overrideGidMax
 	}
 
 	var lookup func(usedGids []int64)
@@ -97,7 +96,7 @@ func getNextUnusedGid(usedGids []int64, gidMin, gidMax int) (nextGid int, err er
 				nextGid = gid
 				return
 			}
-			klog.V(5).Infof("Allocator found GID which is already in use: %v - trying next one.", nextGid)
+			klog.V(5).Infof("Allocator found GID which is already in use: %v, trying next one.", gid)
 		}
 		return
 	}