diff --git a/charts/aws-efs-csi-driver/values.yaml b/charts/aws-efs-csi-driver/values.yaml index e6190f824..56f48c24a 100644 --- a/charts/aws-efs-csi-driver/values.yaml +++ b/charts/aws-efs-csi-driver/values.yaml @@ -178,5 +178,7 @@ storageClasses: [] # 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 98ebded03..6a5656fd6 100644 --- a/docs/README.md +++ b/docs/README.md @@ -26,19 +26,21 @@ 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 | - -**Note** +| 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 the a UID 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 | + +**Note** * 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. * When using a custom Posix group ID range, there is a possibility for the driver to run out of available POSIX group Ids. We suggest ensuring custom group ID range is large enough or create a new storage class with a new file system to provision additional volumes. * `az` under storage class parameter is not be confused with efs-utils mount option `az`. The `az` mount option is used for cross-az mount or efs one zone file system mount within the same aws account as the cluster. @@ -179,13 +181,13 @@ This procedure requires Helm V3 or later. To install or upgrade Helm, see [Using helm repo add aws-efs-csi-driver https://kubernetes-sigs.github.io/aws-efs-csi-driver/ ``` -1. Update the repo. +2. Update the repo. ```sh helm repo update aws-efs-csi-driver ``` -1. Install a release of the driver using the Helm chart. +3. Install a release of the driver using the Helm chart. ```sh helm upgrade --install aws-efs-csi-driver --namespace kube-system aws-efs-csi-driver/aws-efs-csi-driver @@ -232,19 +234,19 @@ If you want to download the image with a manifest, we recommend first trying the **Note** If you encounter an issue that you aren't able to resolve by adding IAM permissions, try the [Manifest \(public registry\)](#-manifest-public-registry-) steps instead. -1. In the following command, replace `region-code` with the AWS Region that your cluster is in. Then run the modified command to replace `us-west-2` in the file with your AWS Region. +2. In the following command, replace `region-code` with the AWS Region that your cluster is in. Then run the modified command to replace `us-west-2` in the file with your AWS Region. ```sh sed -i.bak -e 's|us-west-2|region-code|' private-ecr-driver.yaml ``` -1. Replace `account` in the following command with the account from [Amazon container image registries](add-ons-images.md) for the AWS Region that your cluster is in and then run the modified command to replace `602401143452` in the file. +3. Replace `account` in the following command with the account from [Amazon container image registries](add-ons-images.md) for the AWS Region that your cluster is in and then run the modified command to replace `602401143452` in the file. ```sh sed -i.bak -e 's|602401143452|account|' private-ecr-driver.yaml ``` -1. If you already created a service account by following [Create an IAM policy and role for Amazon EKS](./iam-policy-create.md), then edit the `private-ecr-driver.yaml` file. Remove the following lines that create a Kubernetes service account. +4. If you already created a service account by following [Create an IAM policy and role for Amazon EKS](./iam-policy-create.md), then edit the `private-ecr-driver.yaml` file. Remove the following lines that create a Kubernetes service account. ``` apiVersion: v1 @@ -257,7 +259,7 @@ If you want to download the image with a manifest, we recommend first trying the --- ``` -1. Apply the manifest. +5. Apply the manifest. ```sh kubectl apply -f private-ecr-driver.yaml @@ -277,7 +279,7 @@ For some situations, you may not be able to add the necessary IAM permissions to "github.com/kubernetes-sigs/aws-efs-csi-driver/deploy/kubernetes/overlays/stable/?ref=release-1.X" > public-ecr-driver.yaml ``` -1. If you already created a service account by following [Create an IAM policy and role](./iam-policy-create.md), then edit the `private-ecr-driver.yaml` file. Remove the following lines that create a Kubernetes service account. +2. If you already created a service account by following [Create an IAM policy and role](./iam-policy-create.md), then edit the `private-ecr-driver.yaml` file. Remove the following lines that create a Kubernetes service account. ```sh apiVersion: v1 @@ -290,7 +292,7 @@ For some situations, you may not be able to add the necessary IAM permissions to --- ``` -1. Apply the manifest. +3. Apply the manifest. ```sh kubectl apply -f public-ecr-driver.yaml diff --git a/examples/kubernetes/dynamic_provisioning/README.md b/examples/kubernetes/dynamic_provisioning/README.md index c76b8092b..d94dc3cf6 100644 --- a/examples/kubernetes/dynamic_provisioning/README.md +++ b/examples/kubernetes/dynamic_provisioning/README.md @@ -21,13 +21,13 @@ This example requires Kubernetes 1.17 or later and a driver version of 1.2.0 or fs-582a03f3 ``` - 1. Download a `StorageClass` manifest for Amazon EFS. + 2. Download a `StorageClass` manifest for Amazon EFS. ```sh curl -O https://raw.githubusercontent.com/kubernetes-sigs/aws-efs-csi-driver/master/examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml ``` - 1. Edit [the file](./specs/storageclass.yaml). Find the following line, and replace the value for `fileSystemId` with your file system ID. + 3. Edit [the file](./specs/storageclass.yaml). Find the following line, and replace the value for `fileSystemId` with your file system ID. ``` fileSystemId: fs-582a03f3 @@ -39,14 +39,20 @@ This example requires Kubernetes 1.17 or later and a driver version of 1.2.0 or * `gidRangeStart` (Optional) - The starting range of the Posix group ID to be applied onto the root directory of the access point. The default value is `50000`. * `gidRangeEnd` (Optional) - The ending range of the Posix group ID. The default value is `7000000`. * `basePath` (Optional) - The path on the file system under which the access point root directory is created. If the path isn't provided, the access points root directory is 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 if the pattern were `${.PVC.namespace}/${PVC.name}`, the PVC namespace is `foo` and the PVC name is `pvc-123-456`, and the `basePath` is `/dynamic_provisioner` the access point would be + created at `/dynamic_provisioner/foo/pvc-123-456`. + * `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!** - 1. Deploy the storage class. + 4. Deploy the storage class. ```sh kubectl apply -f storageclass.yaml ``` -1. Test automatic provisioning by deploying a Pod that makes use of the PVC: +2. Test automatic provisioning by deploying a Pod that makes use of the PVC: 1. Download a manifest that deploys a Pod and a PVC. @@ -54,14 +60,12 @@ This example requires Kubernetes 1.17 or later and a driver version of 1.2.0 or curl -O https://raw.githubusercontent.com/kubernetes-sigs/aws-efs-csi-driver/master/examples/kubernetes/dynamic_provisioning/specs/pod.yaml ``` - 1. Deploy the Pod with a sample app and the PVC used by the Pod. + 2. Deploy the Pod with a sample app and the PVC used by the Pod. ```sh kubectl apply -f pod.yaml ``` - -1. Determine the names of the Pods running the controller. - +3. Determine the names of the Pods running the controller. ```sh kubectl get pods -n kube-system | grep efs-csi-controller ``` @@ -73,7 +77,7 @@ This example requires Kubernetes 1.17 or later and a driver version of 1.2.0 or efs-csi-controller-74ccf9f566-wswg9 3/3 Running 0 40m ``` -1. After few seconds, you can observe the controller picking up the change \(edited for readability\). Replace `74ccf9f566-q5989` with a value from one of the Pods in your output from the previous command. +4. After few seconds, you can observe the controller picking up the change \(edited for readability\). Replace `74ccf9f566-q5989` with a value from one of the Pods in your output from the previous command. ```sh kubectl logs efs-csi-controller-74ccf9f566-q5989 \ @@ -91,7 +95,7 @@ This example requires Kubernetes 1.17 or later and a driver version of 1.2.0 or If you don't see the previous output, run the previous command using one of the other controller Pods. -1. Confirm that a persistent volume was created with a status of `Bound` to a `PersistentVolumeClaim`: +5. Confirm that a persistent volume was created with a status of `Bound` to a `PersistentVolumeClaim`: ```sh kubectl get pv @@ -104,7 +108,7 @@ This example requires Kubernetes 1.17 or later and a driver version of 1.2.0 or pvc-5983ffec-96cf-40c1-9cd6-e5686ca84eca 20Gi RWX Delete Bound default/efs-claim efs-sc 7m57s ``` -1. View details about the `PersistentVolumeClaim` that was created. +6. View details about the `PersistentVolumeClaim` that was created. ```sh kubectl get pvc @@ -117,7 +121,7 @@ This example requires Kubernetes 1.17 or later and a driver version of 1.2.0 or efs-claim Bound pvc-5983ffec-96cf-40c1-9cd6-e5686ca84eca 20Gi RWX efs-sc 9m7s ``` -1. View the sample app Pod's status until the `STATUS` becomes `Running`. +7. View the sample app Pod's status until the `STATUS` becomes `Running`. ```sh kubectl get pods -o wide @@ -149,7 +153,7 @@ If a Pod doesn't have an IP address listed, make sure that you added a mount tar [...] ``` -1. \(Optional\) Terminate the Amazon EKS node that your Pod is running on and wait for the Pod to be re\-scheduled. Alternately, you can delete the Pod and redeploy it. Complete the previous step again, confirming that the output includes the previous output. +2. \(Optional\) Terminate the Amazon EKS node that your Pod is running on and wait for the Pod to be re\-scheduled. Alternately, you can delete the Pod and redeploy it. Complete the previous step again, confirming that the output includes the previous output. **Note** When you want to delete an access point in a file system when deleting PVC, you should specify `elasticfilesystem:ClientRootAccess` to the file system access policy to provide the root permissions. diff --git a/examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml b/examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml index 0fcb0cc8f..14c9a8efc 100644 --- a/examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml +++ b/examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml @@ -9,4 +9,6 @@ parameters: 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/go.mod b/go.mod index e79adff93..dd1ebb457 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ require ( github.com/aws/aws-sdk-go v1.44.76 github.com/container-storage-interface/spec v1.6.0 github.com/golang/mock v1.6.0 + github.com/google/uuid v1.3.0 github.com/kubernetes-csi/csi-test/v5 v5.0.0 github.com/mitchellh/go-ps v0.0.0-20170309133038-4fdf99ab2936 github.com/onsi/ginkgo/v2 v2.9.0 @@ -41,7 +42,6 @@ require ( github.com/google/go-cmp v0.5.9 // indirect github.com/google/gofuzz v1.2.0 // indirect github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 // indirect - github.com/google/uuid v1.3.0 // indirect github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect github.com/imdario/mergo v0.3.6 // indirect github.com/inconshreveable/mousetrap v1.0.0 // indirect diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 68b9077f0..78a324b37 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -19,7 +19,10 @@ package driver import ( "context" "fmt" + "github.com/google/uuid" "os" + "path" + "sort" "strconv" "strings" @@ -31,23 +34,28 @@ 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" + 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 +63,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) { @@ -193,10 +208,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 @@ -236,8 +247,41 @@ 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) @@ -476,3 +520,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 fewer + 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 151757b97..8cd57ee32 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() }, }, @@ -1408,6 +1917,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 { @@ -1980,3 +2633,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 +} diff --git a/pkg/driver/sanity_test.go b/pkg/driver/sanity_test.go index 09d9770ca..d1482e63d 100644 --- a/pkg/driver/sanity_test.go +++ b/pkg/driver/sanity_test.go @@ -56,6 +56,7 @@ func TestSanityEFSCSI(t *testing.T) { parameters[FsId] = "fs-1234abcd" parameters[ProvisioningMode] = "efs-ap" parameters[DirectoryPerms] = "777" + parameters[SubPathPattern] = "/foo" config := sanity.NewTestConfig() config.TargetPath = targetPath