diff --git a/charts/aws-efs-csi-driver/values.yaml b/charts/aws-efs-csi-driver/values.yaml index f92205352..178193035 100644 --- a/charts/aws-efs-csi-driver/values.yaml +++ b/charts/aws-efs-csi-driver/values.yaml @@ -132,11 +132,14 @@ storageClasses: [] # mountOptions: # - tls # parameters: +# accessPointID: fsap-0024a40ac730a6cc8 # provisioningMode: efs-ap # fileSystemId: fs-1122aabb # directoryPerms: "700" # gidRangeStart: "1000" # gidRangeEnd: "2000" # basePath: "/dynamic_provisioning" +# subPathPattern: "/subPath" +# ensureUniqueDirectory: true # reclaimPolicy: Delete # volumeBindingMode: Immediate diff --git a/docs/README.md b/docs/README.md index 72132a4ed..869e77635 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1,4 +1,5 @@ [![Build Status](https://travis-ci.org/kubernetes-sigs/aws-efs-csi-driver.svg?branch=master)](https://travis-ci.org/kubernetes-sigs/aws-efs-csi-driver) +[![Build Status](https://travis-ci.org/kubernetes-sigs/aws-efs-csi-driver.svg?branch=master)](https://travis-ci.org/kubernetes-sigs/aws-efs-csi-driver) [![Coverage Status](https://coveralls.io/repos/github/kubernetes-sigs/aws-efs-csi-driver/badge.svg?branch=master)](https://coveralls.io/github/kubernetes-sigs/aws-efs-csi-driver?branch=master) [![Go Report Card](https://goreportcard.com/badge/github.com/kubernetes-sigs/aws-efs-csi-driver)](https://goreportcard.com/report/github.com/kubernetes-sigs/aws-efs-csi-driver) @@ -26,17 +27,19 @@ The following CSI interfaces are implemented: * Identity Service: GetPluginInfo, GetPluginCapabilities, Probe ### Storage Class Parameters for Dynamic Provisioning -| Parameters | Values | Default | Optional | Description | -|---------------------|--------|---------|-----------|-------------| -| provisioningMode | efs-ap | | false | Type of volume provisioned by efs. Currently, Access Points are supported. | -| fileSystemId | | | false | File System under which access points are created. | -| directoryPerms | | | false | Directory permissions for [Access Point root directory](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#enforce-root-directory-access-point) creation. | -| uid | | | true | POSIX user Id to be applied for [Access Point root directory](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#enforce-root-directory-access-point) creation and for [user identity enforcement](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#enforce-identity-access-points). | -| gid | | | true | POSIX group Id to be applied for [Access Point root directory](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#enforce-root-directory-access-point) creation and for [user identity enforcement](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#enforce-identity-access-points). | -| gidRangeStart | | 50000 | true | Start range of the POSIX group Id to be applied for [Access Point root directory](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#enforce-root-directory-access-point) creation and for [user identity enforcement](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#enforce-identity-access-points). Not used if uid/gid is set. For user identity enforcement, this value will be applied as both the uid and the gid. | -| gidRangeEnd | | 7000000 | true | End range of the POSIX group Id. Not used if uid/gid is set. | -| basePath | | | true | Path under which access points for dynamic provisioning is created. If this parameter is not specified, access points are created under the root directory of the file system | -| az | | "" | true | Used for cross-account mount. `az` under storage class parameter is optional. If specified, mount target associated with the az will be used for cross-account mount. If not specified, a random mount target will be picked for cross account mount | +| Parameters | Values | Default | Optional | Description | +|-----------------------|--------|-----------------|----------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| provisioningMode | efs-ap | | false | Type of volume provisioned by efs. Currently, Access Points are supported. | +| fileSystemId | | | false | File System under which access points are created. | +| directoryPerms | | | false | Directory permissions for [Access Point root directory](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#enforce-root-directory-access-point) creation. | +| uid | | | true | POSIX user Id to be applied for [Access Point root directory](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#enforce-root-directory-access-point) creation. | +| gid | | | true | POSIX group Id to be applied for [Access Point root directory](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#enforce-root-directory-access-point) creation. | +| gidRangeStart | | 50000 | true | Start range of the POSIX group Id to be applied for [Access Point root directory](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#enforce-root-directory-access-point) creation. Not used if uid/gid is set. | +| gidRangeEnd | | 7000000 | true | End range of the POSIX group Id. Not used if uid/gid is set. | +| basePath | | | true | Path under which access points for dynamic provisioning is created. If this parameter is not specified, access points are created under the root directory of the file system | +| subPathPattern | | `/${.PV.name}` | true | The template used to construct the subPath under which each of the access points created under Dynamic Provisioning. Can be made up of fixed strings and limited variables, is akin to the 'subPathPattern' variable on the [nfs-subdir-external-provisioner](https://github.com/kubernetes-sigs/nfs-subdir-external-provisioner) chart. Supports `.PVC.name`,`.PVC.namespace` and `.PV.name` | +| ensureUniqueDirectory | | true | true | **NOTE: Only set this to false if you're sure this is the behaviour you want**.
Used when dynamic provisioning is enabled, if set to true, appends a UUID to the pattern specified in `subPathPattern` to ensure that access points will not accidentally point at the same directory. | +| az | | "" | true | Used for cross-account mount. `az` under storage class parameter is optional. If specified, mount target associated with the az will be used for cross-account mount. If not specified, a random mount target will be picked for cross account mount | **Notes**: * Custom Posix group Id range for Access Point root directory must include both `gidRangeStart` and `gidRangeEnd` parameters. These parameters are optional only if both are omitted. If you specify one, the other becomes mandatory. @@ -105,7 +108,7 @@ The following sections are Kubernetes specific. If you are a Kubernetes user, us ### Installation #### Set up driver permission: The driver requires IAM permission to talk to Amazon EFS to manage the volume on user's behalf. There are several methods to grant driver IAM permission: -* Using IAM Role for Service Account (Recommended if you're using EKS): create an [IAM Role for service accounts](https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html) with the [required permissions](./iam-policy-example.json). Uncomment annotations and put the IAM role ARN in [service-account manifest](../deploy/kubernetes/base/serviceaccount-csi-controller.yaml) +* Using IAM Role for Service Account (Recommended if you're using EKS): create an [IAM Role for service accounts](https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html) with the [required permissions](./iam-policy-example.json). Uncomment annotations and put the IAM role ARN in [service-account manifest](../deploy/kubernetes/base/controller-serviceaccount.yaml) * Using IAM [instance profile](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_use_switch-role-ec2_instance-profiles.html) - grant all the worker nodes with [required permissions](./iam-policy-example.json) by attaching policy to the instance profile of the worker. #### Deploy the driver: diff --git a/examples/kubernetes/cross_account_mount/specs/storageclass.yaml b/examples/kubernetes/cross_account_mount/specs/storageclass.yaml index 0ec079aa7..93a5fc04d 100644 --- a/examples/kubernetes/cross_account_mount/specs/storageclass.yaml +++ b/examples/kubernetes/cross_account_mount/specs/storageclass.yaml @@ -4,6 +4,7 @@ metadata: name: efs-sc provisioner: efs.csi.aws.com parameters: + accessPointID: fsap-0024a40ac730a6cc8 # optional provisioningMode: efs-ap fileSystemId: fs-92107410 directoryPerms: "700" diff --git a/examples/kubernetes/dynamic_provisioning/README.md b/examples/kubernetes/dynamic_provisioning/README.md index abcf5b76d..df1fe9439 100644 --- a/examples/kubernetes/dynamic_provisioning/README.md +++ b/examples/kubernetes/dynamic_provisioning/README.md @@ -20,13 +20,23 @@ parameters: gidRangeStart: "1000" gidRangeEnd: "2000" basePath: "/dynamic_provisioning" + subPathPattern: "${.PVC.namespace}/${.PVC.name}" + ensureUniqueDirectories: true ``` * provisioningMode - The type of volume to be provisioned by efs. Currently, only access point based provisioning is supported `efs-ap`. * fileSystemId - The file system under which Access Point is created. * directoryPerms - Directory Permissions of the root directory created by Access Point. * gidRangeStart (Optional) - Starting range of Posix Group ID to be applied onto the root directory of the access point. Default value is 50000. * gidRangeEnd (Optional) - Ending range of Posix Group ID. Default value is 7000000. -* basePath (Optional) - Path on the file system under which access point root directory is created. If path is not provided, access points root directory are created under the root of the file system. +* basePath (Optional) - Path on the file system under which access point root directory is created. If path is not + provided, access points root directory are created under the root of the file system. +* subPathPattern (Optional) - A pattern that describes the subPath under which an access point should be created. So in + the example given above if the PVC namespace is `foo` and the PVC name is `pvc-123-456` the access point would be + created at `/dynamic_provisioner/foo/pvc-123-456-`. (The UUID being appended is due to ensureUniqueDirectories below) +* ensureUniqueDirectories (Optional) - A boolean that ensures that, if set, a UUID is appended to the final element of + any dynamically provisioned path, as in the above example. This can be turned off but this requires you as the + administrator to ensure that your storage classes are set up correctly. Otherwise, it's possible that 2 pods could + end up writing to the same directory by accident. **Please think very carefully before setting this to false!** ### Deploy the Example Create storage class, persistent volume claim (PVC) and the pod which consumes PV: diff --git a/examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml b/examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml index 0fcb0cc8f..6f32a3171 100644 --- a/examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml +++ b/examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml @@ -4,9 +4,12 @@ metadata: name: efs-sc provisioner: efs.csi.aws.com parameters: + accessPointID: fsap-0024a40ac730a6cc8 # optional provisioningMode: efs-ap fileSystemId: fs-92107410 directoryPerms: "700" gidRangeStart: "1000" # optional gidRangeEnd: "2000" # optional - basePath: "/dynamic_provisioning" # optional \ No newline at end of file + basePath: "/dynamic_provisioning" # optional + subPathPattern: "${.PVC.namespace}/${.PVC.name}" # optional + ensureUniqueDirectory: "true" # optional \ No newline at end of file diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index e2b8d147b..a771cf464 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -20,9 +20,13 @@ import ( "context" "fmt" "os" + "path" + "sort" "strconv" "strings" + "github.com/google/uuid" + "github.com/container-storage-interface/spec/lib/go/csi" "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/cloud" "google.golang.org/grpc/codes" @@ -31,23 +35,29 @@ import ( ) const ( - AccessPointMode = "efs-ap" - AzName = "az" - BasePath = "basePath" - DefaultGidMin = 50000 - DefaultGidMax = 7000000 - DefaultTagKey = "efs.csi.aws.com/cluster" - DefaultTagValue = "true" - DirectoryPerms = "directoryPerms" - FsId = "fileSystemId" - Gid = "gid" - GidMin = "gidRangeStart" - GidMax = "gidRangeEnd" - MountTargetIp = "mounttargetip" - ProvisioningMode = "provisioningMode" - RoleArn = "awsRoleArn" - TempMountPathPrefix = "/var/lib/csi/pv" - Uid = "uid" + AccessPointMode = "efs-ap" + AccessPointID = "accessPointId" + AzName = "az" + BasePath = "basePath" + DefaultGidMin = 50000 + DefaultGidMax = 7000000 + DefaultTagKey = "efs.csi.aws.com/cluster" + DefaultTagValue = "true" + DirectoryPerms = "directoryPerms" + EnsureUniqueDirectory = "ensureUniqueDirectory" + FsId = "fileSystemId" + Gid = "gid" + GidMin = "gidRangeStart" + GidMax = "gidRangeEnd" + MountTargetIp = "mounttargetip" + ProvisioningMode = "provisioningMode" + PvName = "csi.storage.k8s.io/pv/name" + PvcName = "csi.storage.k8s.io/pvc/name" + PvcNamespace = "csi.storage.k8s.io/pvc/namespace" + RoleArn = "awsRoleArn" + SubPathPattern = "subPathPattern" + TempMountPathPrefix = "/var/lib/csi/pv" + Uid = "uid" ) var ( @@ -55,6 +65,13 @@ var ( controllerCaps = []csi.ControllerServiceCapability_RPC_Type{ csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, } + // subPathPatternComponents shows the elements that we allow to be in the construction of the root directory + // of the access point, as well as the values we need to extract them from the Volume Parameters. + subPathPatternComponents = map[string]string{ + ".PVC.name": PvcName, + ".PVC.namespace": PvcNamespace, + ".PV.name": PvName, + } ) func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) { @@ -78,6 +95,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) } var ( + accessPointId *string azName string basePath string err error @@ -180,6 +198,11 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) } } + // Check if using single Access Point + if value, ok := volumeParams[AccessPointID]; ok { + accessPointId = &value + } + // Assign default GID ranges if not provided if gidMin == 0 && gidMax == 0 { gidMin = DefaultGidMin @@ -190,10 +213,6 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) accessPointsOptions.DirectoryPerms = value } - if value, ok := volumeParams[BasePath]; ok { - basePath = value - } - // Storage class parameter `az` will be used to fetch preferred mount target for cross account mount. // If the `az` storage class parameter is not provided, a random mount target will be picked for mounting. // This storage class parameter different from `az` mount option provided by efs-utils https://github.com/aws/efs-utils/blob/v1.31.1/src/mount_efs/__init__.py#L195 @@ -233,23 +252,60 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) gid = allocatedGid } + if value, ok := volumeParams[BasePath]; ok { + basePath = value + } + rootDirName := volName - rootDir := basePath + "/" + rootDirName + // Check if a custom structure should be imposed on the access point directory + if value, ok := volumeParams[SubPathPattern]; ok { + // Try and construct the root directory and check it only contains supported components + val, err := interpolateRootDirectoryName(value, volumeParams) + if err == nil { + klog.Infof("Using user-specified structure for access point directory.") + rootDirName = val + if value, ok := volumeParams[EnsureUniqueDirectory]; ok { + if ensureUniqueDirectory, err := strconv.ParseBool(value); !ensureUniqueDirectory && err == nil { + klog.Infof("Not appending PVC UID to path.") + } else { + klog.Infof("Appending PVC UID to path.") + rootDirName = fmt.Sprintf("%s-%s", val, uuid.New().String()) + } + } else { + klog.Infof("Appending PVC UID to path.") + rootDirName = fmt.Sprintf("%s-%s", val, uuid.New().String()) + } + } else { + return nil, err + } + } else { + klog.Infof("Using PV name for access point directory.") + } + + rootDir := path.Join("/", basePath, rootDirName) + if ok, err := validateEfsPathRequirements(rootDir); !ok { + return nil, err + } + klog.Infof("Using %v as the access point directory.", rootDir) accessPointsOptions.Uid = int64(uid) accessPointsOptions.Gid = int64(gid) accessPointsOptions.DirectoryPath = rootDir - accessPointId, err := localCloud.CreateAccessPoint(ctx, volName, accessPointsOptions) - if err != nil { - d.gidAllocator.releaseGid(accessPointsOptions.FileSystemId, gid) - if err == cloud.ErrAccessDenied { - return nil, status.Errorf(codes.Unauthenticated, "Access Denied. Please ensure you have the right AWS permissions: %v", err) - } - if err == cloud.ErrAlreadyExists { - return nil, status.Errorf(codes.AlreadyExists, "Access Point already exists") + if accessPointId == nil { + newAccessPointId, err := localCloud.CreateAccessPoint(ctx, volName, accessPointsOptions) + if err != nil { + d.gidAllocator.releaseGid(accessPointsOptions.FileSystemId, gid) + if err == cloud.ErrAccessDenied { + return nil, status.Errorf(codes.Unauthenticated, "Access Denied. Please ensure you have the right AWS permissions: %v", err) + } + if err == cloud.ErrAlreadyExists { + return nil, status.Errorf(codes.AlreadyExists, "Access Point already exists") + } + return nil, status.Errorf(codes.Internal, "Failed to create Access point in File System %v : %v", accessPointsOptions.FileSystemId, err) } - return nil, status.Errorf(codes.Internal, "Failed to create Access point in File System %v : %v", accessPointsOptions.FileSystemId, err) + + accessPointId = &newAccessPointId.AccessPointId } volContext := map[string]string{} @@ -267,7 +323,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) return &csi.CreateVolumeResponse{ Volume: &csi.Volume{ CapacityBytes: volSize, - VolumeId: accessPointsOptions.FileSystemId + "::" + accessPointId.AccessPointId, + VolumeId: accessPointsOptions.FileSystemId + "::" + *accessPointId, VolumeContext: volContext, }, }, nil @@ -471,3 +527,51 @@ func getCloud(secrets map[string]string, driver *Driver) (cloud.Cloud, string, e return localCloud, roleArn, nil } + +func interpolateRootDirectoryName(rootDirectoryPath string, volumeParams map[string]string) (string, error) { + r := strings.NewReplacer(createListOfVariableSubstitutions(volumeParams)...) + result := r.Replace(rootDirectoryPath) + + // Check if any templating characters still exist + if strings.Contains(result, "${") || strings.Contains(result, "}") { + return "", status.Errorf(codes.InvalidArgument, + "Path specified \"%v\" contains invalid elements. Can only contain %v", rootDirectoryPath, + getSupportedComponentNames()) + } + return result, nil +} + +func createListOfVariableSubstitutions(volumeParams map[string]string) []string { + variableSubstitutions := make([]string, 2*len(subPathPatternComponents)) + i := 0 + for key, volumeParamsKey := range subPathPatternComponents { + variableSubstitutions[i] = "${" + key + "}" + variableSubstitutions[i+1] = volumeParams[volumeParamsKey] + i += 2 + } + return variableSubstitutions +} + +func getSupportedComponentNames() []string { + keys := make([]string, len(subPathPatternComponents)) + + i := 0 + for key := range subPathPatternComponents { + keys[i] = key + i++ + } + sort.Strings(keys) + return keys +} + +func validateEfsPathRequirements(proposedPath string) (bool, error) { + if len(proposedPath) > 100 { + // Check the proposed path is 100 characters or less + return false, status.Errorf(codes.InvalidArgument, "Proposed path '%s' exceeds EFS limit of 100 characters", proposedPath) + } else if strings.Count(proposedPath, "/") > 5 { + // Check the proposed path contains at most 4 subdirectories + return false, status.Errorf(codes.InvalidArgument, "Proposed path '%s' EFS limit of 4 subdirectories", proposedPath) + } else { + return true, nil + } +} diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index 96f4790a9..6b0ddeea4 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -3,37 +3,476 @@ package driver import ( "context" "errors" + "fmt" + "regexp" "testing" + "github.com/google/uuid" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "github.com/container-storage-interface/spec/lib/go/csi" "github.com/golang/mock/gomock" + "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/cloud" "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/driver/mocks" ) -func TestCreateVolume(t *testing.T) { - var ( - endpoint = "endpoint" - volumeName = "volumeName" - fsId = "fs-abcd1234" - apId = "fsap-abcd1234xyz987" - volumeId = "fs-abcd1234::fsap-abcd1234xyz987" - capacityRange int64 = 5368709120 - stdVolCap = &csi.VolumeCapability{ - AccessType: &csi.VolumeCapability_Mount{ - Mount: &csi.VolumeCapability_MountVolume{}, - }, - AccessMode: &csi.VolumeCapability_AccessMode{ - Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, +func TestCreateVolume(t *testing.T) { + var ( + endpoint = "endpoint" + volumeName = "volumeName" + fsId = "fs-abcd1234" + apId = "fsap-abcd1234xyz987" + volumeId = "fs-abcd1234::fsap-abcd1234xyz987" + capacityRange int64 = 5368709120 + stdVolCap = &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, + }, + } + ) + testCases := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Success: Using fixed UID/GID", + 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", + Uid: "1000", + Gid: "1001", + }, + } + + ctx := context.Background() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + } + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + if accessPointOpts.Uid != 1000 { + t.Fatalf("Uid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1000) + } + if accessPointOpts.Gid != 1001 { + t.Fatalf("Gid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1001) + } + }) + + res, err := driver.CreateVolume(ctx, req) + + if err != nil { + t.Fatalf("CreateVolume failed: %v", err) + } + + if res.Volume == nil { + t.Fatal("Volume is nil") + } + + if res.Volume.VolumeId != volumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + } + mockCtl.Finish() + }, + }, + { + name: "Success: Using fixed UID/GID and GID range", + 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: "5000", + GidMax: "10000", + Uid: "1000", + Gid: "1001", + }, + } + + ctx := context.Background() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + } + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + if accessPointOpts.Uid != 1000 { + t.Fatalf("Uid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1000) + } + if accessPointOpts.Gid != 1001 { + t.Fatalf("Gid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1001) + } + }) + + res, err := driver.CreateVolume(ctx, req) + + if err != nil { + t.Fatalf("CreateVolume failed: %v", err) + } + + if res.Volume == nil { + t.Fatal("Volume is nil") + } + + if res.Volume.VolumeId != volumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + } + mockCtl.Finish() + }, + }, + { + name: "Success: Normal flow", + 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{ + stdVolCap, + }, + 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() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + } + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + + res, err := driver.CreateVolume(ctx, req) + + if err != nil { + t.Fatalf("CreateVolume failed: %v", err) + } + + if res.Volume == nil { + t.Fatal("Volume is nil") + } + + if res.Volume.VolumeId != volumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + } + mockCtl.Finish() + }, + }, + { + name: "Success: Using Default GID ranges", + 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", + }, + } + + ctx := context.Background() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + } + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + + res, err := driver.CreateVolume(ctx, req) + + if err != nil { + t.Fatalf("CreateVolume failed: %v", err) + } + + if res.Volume == nil { + t.Fatal("Volume is nil") + } + + if res.Volume.VolumeId != volumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + } + mockCtl.Finish() + }, + }, + { + name: "Success: Normal flow with tags", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + driver := &Driver{ + endpoint: endpoint, + cloud: mockCloud, + gidAllocator: NewGidAllocator(), + tags: parseTagsFromStr("cluster:efs"), + } + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + Parameters: map[string]string{ + ProvisioningMode: "efs-ap", + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + }, + } + + ctx := context.Background() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + } + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + + res, err := driver.CreateVolume(ctx, req) + + if err != nil { + t.Fatalf("CreateVolume failed: %v", err) + } + + if res.Volume == nil { + t.Fatal("Volume is nil") + } + + if res.Volume.VolumeId != volumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + } + mockCtl.Finish() + }, + }, + { + name: "Success: Normal flow with invalid tags", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + driver := &Driver{ + endpoint: endpoint, + cloud: mockCloud, + gidAllocator: NewGidAllocator(), + tags: parseTagsFromStr("cluster-efs"), + } + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + Parameters: map[string]string{ + ProvisioningMode: "efs-ap", + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + }, + } + + ctx := context.Background() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + } + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + + res, err := driver.CreateVolume(ctx, req) + + if err != nil { + t.Fatalf("CreateVolume failed: %v", err) + } + + if res.Volume == nil { + t.Fatal("Volume is nil") + } + + if res.Volume.VolumeId != volumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + } + mockCtl.Finish() + }, + }, + { + name: "Success: Normal flow with a valid directory structure set", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + driver := &Driver{ + endpoint: endpoint, + cloud: mockCloud, + gidAllocator: NewGidAllocator(), + tags: parseTagsFromStr(""), + } + + pvName := "foo" + pvcName := "bar" + directoryCreated := fmt.Sprintf("/%s/%s", pvName, pvcName) + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + Parameters: map[string]string{ + ProvisioningMode: "efs-ap", + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + SubPathPattern: "${.PV.name}/${.PVC.name}", + PvName: pvName, + PvcName: pvcName, + }, + } + + ctx := context.Background() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + } + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + if !verifyPathWhenUUIDIncluded(accessPointOpts.DirectoryPath, directoryCreated) { + t.Fatalf("Root directory mismatch. Expected: %v (with UID appended), actual: %v", + directoryCreated, + accessPointOpts.DirectoryPath) + } + }) + + res, err := driver.CreateVolume(ctx, req) + + if err != nil { + t.Fatalf("CreateVolume failed: %v", err) + } + + if res.Volume == nil { + t.Fatal("Volume is nil") + } + + if res.Volume.VolumeId != volumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + } + + mockCtl.Finish() }, - } - ) - testCases := []struct { - name string - testFunc func(t *testing.T) - }{ + }, { - name: "Success: Using fixed UID/GID", + name: "Success: Normal flow with a valid directory structure set, using a single element", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) @@ -42,8 +481,12 @@ func TestCreateVolume(t *testing.T) { endpoint: endpoint, cloud: mockCloud, gidAllocator: NewGidAllocator(), + tags: parseTagsFromStr(""), } + pvcName := "foo" + directoryCreated := fmt.Sprintf("/%s", pvcName) + req := &csi.CreateVolumeRequest{ Name: volumeName, VolumeCapabilities: []*csi.VolumeCapability{ @@ -55,10 +498,11 @@ func TestCreateVolume(t *testing.T) { Parameters: map[string]string{ ProvisioningMode: "efs-ap", FsId: fsId, + GidMin: "1000", + GidMax: "2000", DirectoryPerms: "777", - BasePath: "test", - Uid: "1000", - Gid: "1001", + SubPathPattern: "${.PVC.name}", + PvcName: pvcName, }, } @@ -71,13 +515,13 @@ func TestCreateVolume(t *testing.T) { FileSystemId: fsId, } mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { - if accessPointOpts.Uid != 1000 { - t.Fatalf("Uid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1000) - } - if accessPointOpts.Gid != 1001 { - t.Fatalf("Gid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1001) + if !verifyPathWhenUUIDIncluded(accessPointOpts.DirectoryPath, directoryCreated) { + t.Fatalf("Root directory mismatch. Expected: %v (with UID appended), actual: %v", + directoryCreated, + accessPointOpts.DirectoryPath) } }) @@ -94,11 +538,12 @@ func TestCreateVolume(t *testing.T) { if res.Volume.VolumeId != volumeId { t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) } + mockCtl.Finish() }, }, { - name: "Success: Using fixed UID/GID and GID range", + name: "Success: Normal flow with a valid directory structure set, and a basePath", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) @@ -107,8 +552,13 @@ func TestCreateVolume(t *testing.T) { endpoint: endpoint, cloud: mockCloud, gidAllocator: NewGidAllocator(), + tags: parseTagsFromStr(""), } + pvcName := "foo" + basePath := "bash" + directoryCreated := fmt.Sprintf("/%s/%s", basePath, pvcName) + req := &csi.CreateVolumeRequest{ Name: volumeName, VolumeCapabilities: []*csi.VolumeCapability{ @@ -118,14 +568,15 @@ func TestCreateVolume(t *testing.T) { RequiredBytes: capacityRange, }, Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - BasePath: "test", - GidMin: "5000", - GidMax: "10000", - Uid: "1000", - Gid: "1001", + ProvisioningMode: "efs-ap", + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + SubPathPattern: "${.PVC.name}", + BasePath: basePath, + EnsureUniqueDirectory: "true", + PvcName: pvcName, }, } @@ -138,13 +589,13 @@ func TestCreateVolume(t *testing.T) { FileSystemId: fsId, } mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { - if accessPointOpts.Uid != 1000 { - t.Fatalf("Uid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1000) - } - if accessPointOpts.Gid != 1001 { - t.Fatalf("Gid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1001) + if !verifyPathWhenUUIDIncluded(accessPointOpts.DirectoryPath, directoryCreated) { + t.Fatalf("Root directory mismatch. Expected: %v (with UID appended), actual: %v", + directoryCreated, + accessPointOpts.DirectoryPath) } }) @@ -161,11 +612,12 @@ func TestCreateVolume(t *testing.T) { if res.Volume.VolumeId != volumeId { t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) } + mockCtl.Finish() }, }, { - name: "Success: Normal flow", + name: "Success: Normal flow with a valid directory structure set, and a basePath, and uniqueness guarantees turned off", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) @@ -177,6 +629,10 @@ func TestCreateVolume(t *testing.T) { tags: parseTagsFromStr(""), } + pvcName := "foo" + basePath := "bash" + directoryCreated := fmt.Sprintf("/%s/%s", basePath, pvcName) + req := &csi.CreateVolumeRequest{ Name: volumeName, VolumeCapabilities: []*csi.VolumeCapability{ @@ -186,12 +642,15 @@ func TestCreateVolume(t *testing.T) { RequiredBytes: capacityRange, }, Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - GidMin: "1000", - GidMax: "2000", - DirectoryPerms: "777", - AzName: "us-east-1a", + ProvisioningMode: "efs-ap", + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + SubPathPattern: "${.PVC.name}", + BasePath: basePath, + EnsureUniqueDirectory: "false", + PvcName: pvcName, }, } @@ -204,7 +663,15 @@ func TestCreateVolume(t *testing.T) { FileSystemId: fsId, } mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + if accessPointOpts.DirectoryPath != directoryCreated { + t.Fatalf("Root directory mismatch. Expected: %v, actual: %v", + directoryCreated, + accessPointOpts.DirectoryPath) + } + }) res, err := driver.CreateVolume(ctx, req) @@ -219,11 +686,13 @@ func TestCreateVolume(t *testing.T) { if res.Volume.VolumeId != volumeId { t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) } + mockCtl.Finish() }, }, { - name: "Success: Using Default GID ranges", + name: "Success: Normal flow with a valid directory structure set, but ensuring uniqueness is set incorrectly, so default of true is used." + + "", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) @@ -232,8 +701,12 @@ func TestCreateVolume(t *testing.T) { endpoint: endpoint, cloud: mockCloud, gidAllocator: NewGidAllocator(), + tags: parseTagsFromStr(""), } + pvcName := "foo" + directoryCreated := fmt.Sprintf("/%s", pvcName) + req := &csi.CreateVolumeRequest{ Name: volumeName, VolumeCapabilities: []*csi.VolumeCapability{ @@ -243,10 +716,14 @@ func TestCreateVolume(t *testing.T) { RequiredBytes: capacityRange, }, Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - BasePath: "test", + ProvisioningMode: "efs-ap", + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + SubPathPattern: "${.PVC.name}", + EnsureUniqueDirectory: "banana", + PvcName: pvcName, }, } @@ -259,7 +736,15 @@ func TestCreateVolume(t *testing.T) { FileSystemId: fsId, } mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + if !verifyPathWhenUUIDIncluded(accessPointOpts.DirectoryPath, directoryCreated) { + t.Fatalf("Root directory mismatch. Expected: %v (with UID appended), actual: %v", + directoryCreated, + accessPointOpts.DirectoryPath) + } + }) res, err := driver.CreateVolume(ctx, req) @@ -274,11 +759,12 @@ func TestCreateVolume(t *testing.T) { if res.Volume.VolumeId != volumeId { t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) } + mockCtl.Finish() }, }, { - name: "Success: Normal flow with tags", + name: "Success: Normal flow with an empty subPath Pattern, no basePath and uniqueness guarantees turned off", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) @@ -287,7 +773,7 @@ func TestCreateVolume(t *testing.T) { endpoint: endpoint, cloud: mockCloud, gidAllocator: NewGidAllocator(), - tags: parseTagsFromStr("cluster:efs"), + tags: parseTagsFromStr(""), } req := &csi.CreateVolumeRequest{ @@ -299,11 +785,13 @@ func TestCreateVolume(t *testing.T) { RequiredBytes: capacityRange, }, Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - GidMin: "1000", - GidMax: "2000", - DirectoryPerms: "777", + ProvisioningMode: "efs-ap", + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + SubPathPattern: "", + EnsureUniqueDirectory: "false", }, } @@ -316,7 +804,15 @@ func TestCreateVolume(t *testing.T) { FileSystemId: fsId, } mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + if accessPointOpts.DirectoryPath != "/" { + t.Fatalf("Root directory mismatch. Expected: %v, actual: %v", + "/", + accessPointOpts.DirectoryPath) + } + }) res, err := driver.CreateVolume(ctx, req) @@ -331,11 +827,12 @@ func TestCreateVolume(t *testing.T) { if res.Volume.VolumeId != volumeId { t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) } + mockCtl.Finish() }, }, { - name: "Success: Normal flow with invalid tags", + name: "Success: Normal flow with an empty subPath Pattern, and basePath set to /", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) @@ -344,7 +841,7 @@ func TestCreateVolume(t *testing.T) { endpoint: endpoint, cloud: mockCloud, gidAllocator: NewGidAllocator(), - tags: parseTagsFromStr("cluster-efs"), + tags: parseTagsFromStr(""), } req := &csi.CreateVolumeRequest{ @@ -356,11 +853,14 @@ func TestCreateVolume(t *testing.T) { RequiredBytes: capacityRange, }, Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - GidMin: "1000", - GidMax: "2000", - DirectoryPerms: "777", + ProvisioningMode: "efs-ap", + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + SubPathPattern: "", + BasePath: "/", + EnsureUniqueDirectory: "false", }, } @@ -373,7 +873,15 @@ func TestCreateVolume(t *testing.T) { FileSystemId: fsId, } mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + if accessPointOpts.DirectoryPath != "/" { + t.Fatalf("Root directory mismatch. Expected: %v, actual: %v", + "/", + accessPointOpts.DirectoryPath) + } + }) res, err := driver.CreateVolume(ctx, req) @@ -388,6 +896,7 @@ func TestCreateVolume(t *testing.T) { if res.Volume.VolumeId != volumeId { t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) } + mockCtl.Finish() }, }, @@ -1360,6 +1869,150 @@ func TestCreateVolume(t *testing.T) { mockCtl.Finish() }, }, + { + name: "Fail: subPathPattern is specified but uses unsupported attributes", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + subPathPattern := "${.PVC.name}/${foo}" + + 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", + SubPathPattern: subPathPattern, + }, + } + + ctx := context.Background() + + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + + _, err := driver.CreateVolume(ctx, req) + if err == nil { + t.Fatal("CreateVolume did not fail") + } + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("Did not throw InvalidArgument error, instead threw %v", err) + } + mockCtl.Finish() + }, + }, + { + name: "Fail: resulting accessPointDirectory is too over 100 characters", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + subPathPattern := "this-directory-name-is-far-too-long-for-any-practical-purposes-and-only-serves-to-prove-a-point" + + 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", + SubPathPattern: subPathPattern, + }, + } + + ctx := context.Background() + + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + + _, err := driver.CreateVolume(ctx, req) + if err == nil { + t.Fatal("CreateVolume did not fail") + } + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("Did not throw InvalidArgument error, instead threw %v", err) + } + mockCtl.Finish() + }, + }, + { + name: "Fail: resulting accessPointDirectory contains over 4 subdirectories", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + subPathPattern := "a/b/c/d/e/f" + + 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", + SubPathPattern: subPathPattern, + }, + } + + ctx := context.Background() + + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + + _, err := driver.CreateVolume(ctx, req) + if err == nil { + t.Fatal("CreateVolume did not fail") + } + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("Did not throw InvalidArgument error, instead threw %v", err) + } + mockCtl.Finish() + }, + }, } for _, tc := range testCases { @@ -1932,3 +2585,11 @@ func TestControllerGetCapabilities(t *testing.T) { t.Fatalf("ControllerGetCapabilities failed: %v", err) } } + +func verifyPathWhenUUIDIncluded(pathToVerify string, expectedPathWithoutUUID string) bool { + r := regexp.MustCompile("(.*)-([0-9A-fA-F]+-[0-9A-fA-F]+-[0-9A-fA-F]+-[0-9A-fA-F]+-[0-9A-fA-F]+$)") + matches := r.FindStringSubmatch(pathToVerify) + doesPathMatchWithUuid := matches[1] == expectedPathWithoutUUID + _, err := uuid.Parse(matches[2]) + return err == nil && doesPathMatchWithUuid +}