diff --git a/charts/aws-efs-csi-driver/templates/controller-deployment.yaml b/charts/aws-efs-csi-driver/templates/controller-deployment.yaml index ab9dbb6c2..db66c9e81 100644 --- a/charts/aws-efs-csi-driver/templates/controller-deployment.yaml +++ b/charts/aws-efs-csi-driver/templates/controller-deployment.yaml @@ -61,6 +61,7 @@ spec: {{- end }} - --v={{ .Values.controller.logLevel }} - --delete-access-point-root-dir={{ hasKey .Values.controller "deleteAccessPointRootDir" | ternary .Values.controller.deleteAccessPointRootDir false }} + - --delete-provisioned-dir={{ hasKey .Values.controller "deleteProvisionedDir" | ternary .Values.controller.deleteProvisionedDir false }} env: - name: CSI_ENDPOINT value: unix:///var/lib/csi/sockets/pluginproxy/csi.sock diff --git a/charts/aws-efs-csi-driver/values.yaml b/charts/aws-efs-csi-driver/values.yaml index e6190f824..e5f034acd 100644 --- a/charts/aws-efs-csi-driver/values.yaml +++ b/charts/aws-efs-csi-driver/values.yaml @@ -60,8 +60,10 @@ controller: # environment: prod # region: us-east-1 # Enable if you want the controller to also delete the - # path on efs when deleteing an access point + # path on efs when deleting an EFS access point deleteAccessPointRootDir: false + # Enable if you want the controller to delete any directories it also provisions + deleteProvisionedDir: false podAnnotations: {} resources: {} diff --git a/cmd/main.go b/cmd/main.go index a74eaecf8..6a64dfed1 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -41,6 +41,8 @@ func main() { volMetricsFsRateLimit = flag.Int("vol-metrics-fs-rate-limit", 5, "Volume metrics routines rate limiter per file system") deleteAccessPointRootDir = flag.Bool("delete-access-point-root-dir", false, "Opt in to delete access point root directory by DeleteVolume. By default, DeleteVolume will delete the access point behind Persistent Volume and deleting access point will not delete the access point root directory or its contents.") + deleteProvisionedDir = flag.Bool("delete-provisioned-dir", false, + "Opt in to delete any provisioned directories and their contents. By default, DeleteVolume will not delete the directory behind Persistent Volume. Note: If root squash is enabled on your EFS it's possible this option will not work.") tags = flag.String("tags", "", "Space separated key:value pairs which will be added as tags for EFS resources. For example, 'environment:prod region:us-east-1'") ) klog.InitFlags(nil) @@ -60,7 +62,7 @@ func main() { if err != nil { klog.Fatalln(err) } - drv := driver.NewDriver(*endpoint, etcAmazonEfs, *efsUtilsStaticFilesPath, *tags, *volMetricsOptIn, *volMetricsRefreshPeriod, *volMetricsFsRateLimit, *deleteAccessPointRootDir) + drv := driver.NewDriver(*endpoint, etcAmazonEfs, *efsUtilsStaticFilesPath, *tags, *volMetricsOptIn, *volMetricsRefreshPeriod, *volMetricsFsRateLimit, *deleteAccessPointRootDir, *deleteProvisionedDir) if err := drv.Run(); err != nil { klog.Fatalln(err) } diff --git a/docs/README.md b/docs/README.md index 98ebded03..2a04230f0 100644 --- a/docs/README.md +++ b/docs/README.md @@ -26,19 +26,25 @@ 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/efs-dir | | false | Type of volume provisioned by efs. `efs-ap` will provision EFS Access Points, while `efs-dir` will provision directories on the EFS instead. | +| 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 | +| 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** +* We suggest the following settings for each provisioning mode: + * `efs-ap` -> `directoryPerms: 770`, with `gidRangeStart` and `gidRangeEnd` set to sensible values + * `efs-dir` -> `directoryPerms: 777` and omit `gidRangeStart` and `gidRangeEnd` + * This ensures that important system utilities will still be to access to the directories created + * All parameters are available in all modes but please think very carefully about what you're doing otherwise directories could be created that no-one can read or write to, causing strange bugs in the driver. If you need this feature we recommend you maintain a break-glass root user that can clear up any directories + that get created like this. * 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. @@ -128,7 +134,7 @@ You can find previous efs-csi-driver versions' images from [here](https://galler ### Features * Static provisioning - Amazon EFS file system needs to be created manually first, then it could be mounted inside container as a persistent volume (PV) using the driver. -* Dynamic provisioning - Uses a persistent volume claim (PVC) to dynamically provision a persistent volume (PV). On Creating a PVC, kuberenetes requests Amazon EFS to create an Access Point in a file system which will be used to mount the PV. +* Dynamic provisioning - Uses a persistent volume claim (PVC) to dynamically provision a persistent volume (PV). On Creating a PVC, Kubernetes requests Amazon EFS to create an Access Point or a Directory in the file system which will be used to mount the PV. * Mount Options - Mount options can be specified in the persistent volume (PV) or storage class for dynamic provisioning to define how the volume should be mounted. * Encryption of data in transit - Amazon EFS file systems are mounted with encryption in transit enabled by default in the master branch version of the driver. * Cross account mount - Amazon EFS file systems from different aws accounts can be mounted from an Amazon EKS cluster. @@ -179,13 +185,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 +238,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 +263,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 +283,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 +296,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 @@ -302,17 +308,19 @@ After deploying the driver, you can continue to these sections: * [Examples](#examples) ### Container Arguments for efs-plugin of efs-csi-node daemonset -| Parameters | Values | Default | Optional | Description | -|-----------------------------|--------|---------|----------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| vol-metrics-opt-in | | false | true | Opt in to emit volume metrics. | -| vol-metrics-refresh-period | | 240 | true | Refresh period for volume metrics in minutes. | -| vol-metrics-fs-rate-limit | | 5 | true | Volume metrics routines rate limiter per file system. | -| tags | | | true | Space separated key:value pairs which will be added as tags for Amazon EFS resources. For example, '--tags=name:efs-tag-test date:Jan24' | +| Parameters | Values | Default | Optional | Description | +|----------------------------|--------|---------|----------|------------------------------------------------------------------------------------------------------------------------------------------| +| vol-metrics-opt-in | | false | true | Opt in to emit volume metrics. | +| vol-metrics-refresh-period | | 240 | true | Refresh period for volume metrics in minutes. | +| vol-metrics-fs-rate-limit | | 5 | true | Volume metrics routines rate limiter per file system. | +| tags | | | true | Space separated key:value pairs which will be added as tags for Amazon EFS resources. For example, '--tags=name:efs-tag-test date:Jan24' | ### Container Arguments for deployment(controller) -| Parameters | Values | Default | Optional | Description | -|-----------------------------|--------|---------|----------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| delete-access-point-root-dir| | false | true | Opt in to delete access point root directory by DeleteVolume. By default, DeleteVolume will delete the access point behind Persistent Volume and deleting access point will not delete the access point root directory or its contents. | +| Parameters | Values | Default | Optional | Description | +|------------------------------|--------|---------|----------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| delete-access-point-root-dir | | false | true | Opt in to delete access point root directory by DeleteVolume. By default, DeleteVolume will delete the access point behind Persistent Volume and deleting access point will not delete the access point root directory or its contents. | +| delete-provisioned-dir | | false | true | Opt in to delete any provisioned directories and their contents. By default, DeleteVolume will not delete the directory behind Persistent Volume. *NOTE*: If root squash is enabled on your EFS it's possible this option will not work, unless you set up IAM roles to circumvent the root-squashing. See [here](https://docs.aws.amazon.com/efs/latest/ug/enable-root-squashing.html) for more details | + ### Upgrading the Amazon EFS CSI Driver @@ -341,7 +349,8 @@ Before following the examples, you need to: #### Example links * [Static provisioning](../examples/kubernetes/static_provisioning/README.md) -* [Dynamic provisioning](../examples/kubernetes/dynamic_provisioning/README.md) +* [Dynamic provisioning with Access Points](../examples/kubernetes/dynamic_provisioning/access_points/README.md) +* [Dynamic provisioning with Directories](../examples/kubernetes/dynamic_provisioning/directories/README.md) * [Encryption in transit](../examples/kubernetes/encryption_in_transit/README.md) * [Accessing the file system from multiple pods](../examples/kubernetes/multiple_pods/README.md) * [Consume Amazon EFS in StatefulSets](../examples/kubernetes/statefulset/README.md) diff --git a/examples/kubernetes/dynamic_provisioning/README.md b/examples/kubernetes/dynamic_provisioning/access_points/README.md similarity index 99% rename from examples/kubernetes/dynamic_provisioning/README.md rename to examples/kubernetes/dynamic_provisioning/access_points/README.md index c76b8092b..13cd3f709 100644 --- a/examples/kubernetes/dynamic_provisioning/README.md +++ b/examples/kubernetes/dynamic_provisioning/access_points/README.md @@ -51,7 +51,7 @@ This example requires Kubernetes 1.17 or later and a driver version of 1.2.0 or 1. Download a manifest that deploys a Pod and a PVC. ```sh - curl -O https://raw.githubusercontent.com/kubernetes-sigs/aws-efs-csi-driver/master/examples/kubernetes/dynamic_provisioning/specs/pod.yaml + curl -O https://raw.githubusercontent.com/kubernetes-sigs/aws-efs-csi-driver/master/examples/kubernetes/dynamic_provisioning/access_points/specs/pod.yaml ``` 1. Deploy the Pod with a sample app and the PVC used by the Pod. diff --git a/examples/kubernetes/dynamic_provisioning/specs/pod.yaml b/examples/kubernetes/dynamic_provisioning/access_points/specs/pod.yaml similarity index 100% rename from examples/kubernetes/dynamic_provisioning/specs/pod.yaml rename to examples/kubernetes/dynamic_provisioning/access_points/specs/pod.yaml diff --git a/examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml b/examples/kubernetes/dynamic_provisioning/access_points/specs/storageclass.yaml similarity index 100% rename from examples/kubernetes/dynamic_provisioning/specs/storageclass.yaml rename to examples/kubernetes/dynamic_provisioning/access_points/specs/storageclass.yaml diff --git a/examples/kubernetes/dynamic_provisioning/directories/README.md b/examples/kubernetes/dynamic_provisioning/directories/README.md new file mode 100644 index 000000000..98f04c5b3 --- /dev/null +++ b/examples/kubernetes/dynamic_provisioning/directories/README.md @@ -0,0 +1,152 @@ +## Dynamic Provisioning +**Important** +You can't use dynamic provisioning with Fargate nodes. + +This example shows how to create a dynamically provisioned volume created through a directory on the file system and a Persistent Volume Claim (PVC) and consume it from a pod. + +**Prerequisite** +This example requires Kubernetes 1.17 or later and a driver version of 1.5.x or later. + +1. Create a storage class for Amazon EFS. + + 1. Retrieve your Amazon EFS file system ID. You can find this in the Amazon EFS console, or use the following AWS CLI command. + + ```sh + aws efs describe-file-systems --query "FileSystems[*].FileSystemId" --output text + ``` + + The example output is as follows. + + ``` + fs-582a03f3 + ``` + + 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/directories/specs/storageclass.yaml + ``` + + 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 + ``` + + Modify the other values as needed: + * `fileSystemId` - The file system under which the access point is created. + * `directoryPerms` - The directory permissions of the root directory created by the access point. + * `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. + + 4. Deploy the storage class. + + ```sh + kubectl apply -f storageclass.yaml + ``` + +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. + + ```sh + curl -O https://raw.githubusercontent.com/kubernetes-sigs/aws-efs-csi-driver/master/examples/kubernetes/dynamic_provisioning/specs/pod.yaml + ``` + + 2. Deploy the Pod with a sample app and the PVC used by the Pod. + + ```sh + kubectl apply -f pod.yaml + ``` + +3. Determine the names of the Pods running the controller. + + ```sh + kubectl get pods -n kube-system | grep efs-csi-controller + ``` + + The example output is as follows. + + ``` + efs-csi-controller-74ccf9f566-q5989 3/3 Running 0 40m + efs-csi-controller-74ccf9f566-wswg9 3/3 Running 0 40m + ``` + +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 \ + -n kube-system \ + -c csi-provisioner \ + --tail 10 + ``` + + The example output is as follows. + + ``` + [...] + 1 controller.go:737] successfully created PV pvc-5983ffec-96cf-40c1-9cd6-e5686ca84eca for PVC efs-claim... + ``` + + If you don't see the previous output, run the previous command using one of the other controller Pods. + +5. Confirm that a persistent volume was created with a status of `Bound` to a `PersistentVolumeClaim`: + + ```sh + kubectl get pv + ``` + + The example output is as follows. + + ``` + NAME CAPACITY ACCESS MODES RECLAIM POLICY STATUS CLAIM STORAGECLASS REASON AGE + pvc-5983ffec-96cf-40c1-9cd6-e5686ca84eca 20Gi RWX Delete Bound default/efs-claim efs-sc 7m57s + ``` + +6. View details about the `PersistentVolumeClaim` that was created. + + ```sh + kubectl get pvc + ``` + + The example output is as follows. + + ``` + NAME STATUS VOLUME CAPACITY ACCESS MODES STORAGECLASS AGE + efs-claim Bound pvc-5983ffec-96cf-40c1-9cd6-e5686ca84eca 20Gi RWX efs-sc 9m7s + ``` + +7. View the sample app Pod's status until the `STATUS` becomes `Running`. + + ```sh + kubectl get pods -o wide + ``` + + The example output is as follows. + + ``` + NAME READY STATUS RESTARTS AGE IP NODE NOMINATED NODE READINESS GATES + efs-app 1/1 Running 0 10m 192.168.78.156 ip-192-168-73-191.region-code.compute.internal + ``` +**Note** +If a Pod doesn't have an IP address listed, make sure that you added a mount target for the subnet that your node is in \(as described at the end of [Create an Amazon EFS file system](#efs-create-filesystem)\). Otherwise the Pod won't leave `ContainerCreating` status. When an IP address is listed, it may take a few minutes for a Pod to reach the `Running` status. + +1. Confirm that the data is written to the volume. + + ```sh + kubectl exec efs-app -- bash -c "cat data/out" + ``` + + The example output is as follows. + + ``` + [...] + Tue Mar 23 14:29:16 UTC 2021 + Tue Mar 23 14:29:21 UTC 2021 + Tue Mar 23 14:29:26 UTC 2021 + Tue Mar 23 14:29:31 UTC 2021 + [...] + ``` + +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. \ No newline at end of file diff --git a/examples/kubernetes/dynamic_provisioning/directories/specs/pod.yaml b/examples/kubernetes/dynamic_provisioning/directories/specs/pod.yaml new file mode 100644 index 000000000..522a690e1 --- /dev/null +++ b/examples/kubernetes/dynamic_provisioning/directories/specs/pod.yaml @@ -0,0 +1,30 @@ +--- +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: efs-claim +spec: + accessModes: + - ReadWriteMany + storageClassName: efs-sc + resources: + requests: + storage: 5Gi +--- +apiVersion: v1 +kind: Pod +metadata: + name: efs-app +spec: + containers: + - name: app + image: centos + command: ["/bin/sh"] + args: ["-c", "while true; do echo $(date -u) >> /data/out; sleep 5; done"] + volumeMounts: + - name: persistent-storage + mountPath: /data + volumes: + - name: persistent-storage + persistentVolumeClaim: + claimName: efs-claim \ No newline at end of file diff --git a/examples/kubernetes/dynamic_provisioning/directories/specs/storageclass.yaml b/examples/kubernetes/dynamic_provisioning/directories/specs/storageclass.yaml new file mode 100644 index 000000000..e3f91ec89 --- /dev/null +++ b/examples/kubernetes/dynamic_provisioning/directories/specs/storageclass.yaml @@ -0,0 +1,12 @@ +kind: StorageClass +apiVersion: storage.k8s.io/v1 +metadata: + name: efs-sc +provisioner: efs.csi.aws.com +parameters: + provisioningMode: efs-dir + fileSystemId: fs-92107410 + directoryPerms: "700" + gidRangeStart: "1000" # optional + gidRangeEnd: "2000" # optional + basePath: "/dynamic_provisioning" # optional \ No newline at end of file diff --git a/hack/values.yaml b/hack/values.yaml index f863f750e..ef9bd720e 100644 --- a/hack/values.yaml +++ b/hack/values.yaml @@ -1,8 +1,9 @@ controller: create: true logLevel: 5 + serviceAccount: + create: true + deleteProvisionedDir: true node: logLevel: 5 -serviceAccount: - controller: - create: true + diff --git a/hack/values_eksctl.yaml b/hack/values_eksctl.yaml index f99eec38d..d18e1b1df 100644 --- a/hack/values_eksctl.yaml +++ b/hack/values_eksctl.yaml @@ -2,6 +2,7 @@ controller: logLevel: 5 serviceAccount: create: false # let eksctl create it + deleteProvisionedDir: true node: logLevel: 5 serviceAccount: diff --git a/pkg/driver/access_point_provisioner.go b/pkg/driver/access_point_provisioner.go new file mode 100644 index 000000000..383fa89d4 --- /dev/null +++ b/pkg/driver/access_point_provisioner.go @@ -0,0 +1,223 @@ +package driver + +import ( + "context" + "os" + "strings" + + "github.com/container-storage-interface/spec/lib/go/csi" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "k8s.io/klog/v2" + + "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/cloud" +) + +type AccessPointProvisioner struct { + tags map[string]string + cloud cloud.Cloud + deleteAccessPointRootDir bool + mounter Mounter +} + +func (a AccessPointProvisioner) Provision(ctx context.Context, req *csi.CreateVolumeRequest, uid, gid int) (*csi.Volume, error) { + volumeParams := req.GetParameters() + volName := req.GetName() + if volName == "" { + return nil, status.Error(codes.InvalidArgument, "Volume name not provided") + } + + var ( + azName string + err error + roleArn string + ) + + // Volume size is required to match PV to PVC by k8s. + // Volume size is not consumed by EFS for any purposes. + volSize := req.GetCapacityRange().GetRequiredBytes() + + accessPointsOptions, err := a.deriveAccessPointOptions(req, uid, gid) + + localCloud, roleArn, err := getCloud(a.cloud, req.GetSecrets()) + if err != nil { + return nil, err + } + + // 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 + // The `az` mount option provided by efs-utils is used for cross az mount or to provide az of efs one zone file system mount within the same aws-account. + // To make use of the `az` mount option, add it under storage class's `mountOptions` section. https://kubernetes.io/docs/concepts/storage/storage-classes/#mount-options + if value, ok := volumeParams[AzName]; ok { + azName = value + } + + // Check if file system exists. Describe FS handles appropriate error codes + if _, err = localCloud.DescribeFileSystem(ctx, accessPointsOptions.FileSystemId); err != nil { + 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.ErrNotFound { + return nil, status.Errorf(codes.InvalidArgument, "File System does not exist: %v", err) + } + return nil, status.Errorf(codes.Internal, "Failed to fetch File System info: %v", err) + } + + accessPointId, err := localCloud.CreateAccessPoint(ctx, volName, accessPointsOptions) + if err != nil { + 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) + } + + volContext := map[string]string{} + + // Fetch mount target Ip for cross-account mount + if roleArn != "" { + mountTarget, err := localCloud.DescribeMountTargets(ctx, accessPointsOptions.FileSystemId, azName) + if err != nil { + klog.Warningf("Failed to describe mount targets for file system %v. Skip using `mounttargetip` mount option: %v", accessPointsOptions.FileSystemId, err) + } else { + volContext[MountTargetIp] = mountTarget.IPAddress + } + } + + return &csi.Volume{ + CapacityBytes: volSize, + VolumeId: accessPointsOptions.FileSystemId + "::" + accessPointId.AccessPointId, + VolumeContext: volContext, + }, nil +} + +func (a AccessPointProvisioner) deriveAccessPointOptions(req *csi.CreateVolumeRequest, + uid int, gid int) (*cloud.AccessPointOptions, error) { + + accessPointsOptions := &cloud.AccessPointOptions{ + CapacityGiB: req.GetCapacityRange().GetRequiredBytes(), + Tags: a.getTags(), + Uid: int64(uid), + Gid: int64(gid), + } + + volumeParams := req.Parameters + + if value, ok := volumeParams[FsId]; ok { + if strings.TrimSpace(value) == "" { + return nil, status.Errorf(codes.InvalidArgument, "Parameter %v cannot be empty", FsId) + } + accessPointsOptions.FileSystemId = value + } else { + return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", FsId) + } + + if value, ok := volumeParams[DirectoryPerms]; ok { + accessPointsOptions.DirectoryPerms = value + } + + var basePath string + if value, ok := volumeParams[BasePath]; ok { + basePath = value + } + + rootDirName := req.Name + rootDir := basePath + "/" + rootDirName + accessPointsOptions.DirectoryPath = rootDir + + return accessPointsOptions, nil +} + +func (a AccessPointProvisioner) getTags() map[string]string { + // Create tags + tags := map[string]string{ + DefaultTagKey: DefaultTagValue, + } + + // Append input tags to default tag + if len(a.tags) != 0 { + for k, v := range a.tags { + tags[k] = v + } + } + return tags +} + +func (a AccessPointProvisioner) Delete(ctx context.Context, req *csi.DeleteVolumeRequest) error { + localCloud, roleArn, err := getCloud(a.cloud, req.GetSecrets()) + if err != nil { + return err + } + + fileSystemId, _, accessPointId, _ := parseVolumeId(req.GetVolumeId()) + if accessPointId != "" { + // Delete access point root directory if delete-access-point-root-dir is set. + if a.deleteAccessPointRootDir { + // Check if Access point exists. + // If access point exists, retrieve its root directory and delete it/ + accessPoint, err := localCloud.DescribeAccessPoint(ctx, accessPointId) + if err != nil { + if err == cloud.ErrAccessDenied { + return status.Errorf(codes.Unauthenticated, "Access Denied. Please ensure you have the right AWS permissions: %v", err) + } + if err == cloud.ErrNotFound { + klog.V(5).Infof("DeleteVolume: Access Point %v not found, returning success", accessPointId) + return nil + } + return status.Errorf(codes.Internal, "Could not get describe Access Point: %v , error: %v", accessPointId, err) + } + + //Mount File System at it root and delete access point root directory + mountOptions := []string{"tls", "iam"} + if roleArn != "" { + mountTarget, err := localCloud.DescribeMountTargets(ctx, fileSystemId, "") + + if err == nil { + mountOptions = append(mountOptions, MountTargetIp+"="+mountTarget.IPAddress) + } else { + klog.Warningf("Failed to describe mount targets for file system %v. Skip using `mounttargetip` mount option: %v", fileSystemId, err) + } + } + + target := TempMountPathPrefix + "/" + accessPointId + if err := a.mounter.MakeDir(target); err != nil { + return status.Errorf(codes.Internal, "Could not create dir %q: %v", target, err) + } + if err := a.mounter.Mount(fileSystemId, target, "efs", mountOptions); err != nil { + os.Remove(target) + return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", fileSystemId, target, err) + } + err = os.RemoveAll(target + accessPoint.AccessPointRootDir) + if err != nil { + return status.Errorf(codes.Internal, "Could not delete access point root directory %q: %v", accessPoint.AccessPointRootDir, err) + } + err = a.mounter.Unmount(target) + if err != nil { + return status.Errorf(codes.Internal, "Could not unmount %q: %v", target, err) + } + err = os.RemoveAll(target) + if err != nil { + return status.Errorf(codes.Internal, "Could not delete %q: %v", target, err) + } + } + + // Delete access point + if err = localCloud.DeleteAccessPoint(ctx, accessPointId); err != nil { + if err == cloud.ErrAccessDenied { + return status.Errorf(codes.Unauthenticated, "Access Denied. Please ensure you have the right AWS permissions: %v", err) + } + if err == cloud.ErrNotFound { + klog.V(5).Infof("DeleteVolume: Access Point not found, returning success") + return nil + } + return status.Errorf(codes.Internal, "Failed to Delete volume %v: %v", req.GetVolumeId(), err) + } + } else { + return status.Errorf(codes.NotFound, "Failed to find access point for volume: %v", req.GetVolumeId()) + } + + return nil +} diff --git a/pkg/driver/access_point_provisioner_test.go b/pkg/driver/access_point_provisioner_test.go new file mode 100644 index 000000000..cb05e9cfc --- /dev/null +++ b/pkg/driver/access_point_provisioner_test.go @@ -0,0 +1,782 @@ +package driver + +import ( + "context" + "errors" + "fmt" + "reflect" + "testing" + + "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 TestAccessPointProvisioner_DeriveAccessPointOptions(t *testing.T) { + var ( + fsId = "fs-abcd1234" + volumeName = "volumeName" + 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, + }, + } + ) + + tests := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Success: Default tags are respected", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + 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", + }, + } + + tags := map[string]string{ + "cluster": "efs", + } + apProv := AccessPointProvisioner{ + tags: tags, + cloud: mockCloud, + deleteAccessPointRootDir: false, + mounter: nil, + } + + apOpts, _ := apProv.deriveAccessPointOptions(req, 1000, 1000) + + expectedTags := make(map[string]string, len(tags)+1) + for k, v := range tags { + expectedTags[k] = v + } + expectedTags[DefaultTagKey] = DefaultTagValue + + if !reflect.DeepEqual(apOpts.Tags, expectedTags) { + t.Fatalf("Expected tags to be %v, but was %v", expectedTags, apOpts.Tags) + } + + mockCtl.Finish() + }, + }, + { + name: "Fail: Missing Parameter FsId", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + Parameters: map[string]string{ + ProvisioningMode: "efs-ap", + DirectoryPerms: "777", + }, + } + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: false, + mounter: nil, + } + + _, err := apProv.deriveAccessPointOptions(req, 1000, 1000) + + if err == nil { + t.Fatal("Expected deriveAccessPoints to fail but it didn't") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: FsId cannot be blank", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + Parameters: map[string]string{ + ProvisioningMode: "efs-ap", + FsId: " ", + DirectoryPerms: "777", + }, + } + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: false, + mounter: nil, + } + + _, err := apProv.deriveAccessPointOptions(req, 1000, 1000) + + if err == nil { + t.Fatal("Expected deriveAccessPoints to fail but it didn't") + } + mockCtl.Finish() + }, + }, + } + for _, test := range tests { + t.Run(test.name, test.testFunc) + } +} + +func TestAccessPointProvisioner_Provision(t *testing.T) { + var ( + fsId = "fs-abcd1234" + volumeName = "volumeName" + 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, + }, + } + ) + tests := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Fail: File system does not exist", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + ctx := context.Background() + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(nil, cloud.ErrNotFound) + + 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", + }, + } + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: false, + mounter: nil, + } + + _, err := apProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected Provision to fail but it didn't") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: DescribeFileSystem Access Denied", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + ctx := context.Background() + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(nil, cloud.ErrAccessDenied) + + 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", + }, + } + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: false, + mounter: nil, + } + + _, err := apProv.Provision(ctx, req, 1000, 1000) + if err == nil { + t.Fatal("Expected Provision to fail but it didn't") + } + + mockCtl.Finish() + }, + }, + { + name: "Fail: Describe File system call fails", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + ctx := context.Background() + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(nil, errors.New("DescribeFileSystem failed")) + + 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", + }, + } + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: false, + mounter: nil, + } + + _, err := apProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected Provision to fail but it didn't") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Create Access Point call fails", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + ctx := context.Background() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(nil, errors.New("CreateAccessPoint call failed")) + + 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", + }, + } + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: false, + mounter: nil, + } + + _, err := apProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected Provision to fail but it didn't") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: CreateAccessPoint Access Denied", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + ctx := context.Background() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(nil, cloud.ErrAccessDenied) + + 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", + }, + } + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: false, + mounter: nil, + } + + _, err := apProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected Provision to fail but it didn't") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Cannot assume role for x-account", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + secrets := map[string]string{} + secrets["awsRoleArn"] = "arn:aws:iam::1234567890:role/EFSCrossAccountRole" + + 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", + }, + Secrets: secrets, + } + + ctx := context.Background() + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: false, + mounter: nil, + } + + _, err := apProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected Provision to fail but it didn't") + } + + mockCtl.Finish() + }, + }, + } + for _, test := range tests { + t.Run(test.name, test.testFunc) + } +} + +func TestAccessPointProvisioner_Delete(t *testing.T) { + var ( + fsId = "fs-abcd1234" + apId = "fsap-abcd1234xyz987" + volumeId = fmt.Sprintf("%s::%s", fsId, apId) + ) + + tests := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Success: Setting deleteAccessPointRootDir causes rootDir to be deleted", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + AccessPointRootDir: "", + CapacityGiB: 0, + } + + ctx := context.Background() + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockMounter.EXPECT().Unmount(gomock.Any()).Return(nil) + mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(accessPoint, nil) + mockCloud.EXPECT().DeleteAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(nil) + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: true, + mounter: mockMounter, + } + + err := apProv.Delete(ctx, req) + + if err != nil { + t.Fatalf("Expected Delete to succeed but it failed: %v", err) + } + mockCtl.Finish() + }, + }, + { + name: "Success: If AccessPoint does not exist success is returned as no work needs to be done", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + ctx := context.Background() + mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(nil, cloud.ErrNotFound) + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: true, + mounter: mockMounter, + } + + err := apProv.Delete(ctx, req) + + if err != nil { + t.Fatalf("Expected Delete to succeed but it failed: %v", err) + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Return error if AccessDenied error from AWS", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + ctx := context.Background() + mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(nil, cloud.ErrAccessDenied) + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: true, + mounter: mockMounter, + } + + err := apProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected Delete to fail but it succeeded") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Return error if DescribeAccessPoints failed", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + ctx := context.Background() + mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(nil, errors.New("Describe Access Point failed")) + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: true, + mounter: mockMounter, + } + + err := apProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected Delete to fail but it succeeded") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Fail to make directory for access point mount", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + AccessPointRootDir: "", + CapacityGiB: 0, + } + + ctx := context.Background() + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(errors.New("Failed to makeDir")) + mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(accessPoint, nil) + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: true, + mounter: mockMounter, + } + + err := apProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected Delete to fail but it succeeded") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Fail to mount file system on directory for access point root directory removal", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + AccessPointRootDir: "", + CapacityGiB: 0, + } + + ctx := context.Background() + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("Failed to mount")) + mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(accessPoint, nil) + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: true, + mounter: mockMounter, + } + + err := apProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected Delete to fail but it succeeded") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Fail to unmount file system after access point root directory removal", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + AccessPointRootDir: "", + CapacityGiB: 0, + } + + ctx := context.Background() + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockMounter.EXPECT().Unmount(gomock.Any()).Return(errors.New("Failed to unmount")) + mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(accessPoint, nil) + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: true, + mounter: mockMounter, + } + + err := apProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected Delete to fail but it succeeded") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: DeleteAccessPoint access denied", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + ctx := context.Background() + mockCloud.EXPECT().DeleteAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(cloud.ErrAccessDenied) + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: false, + mounter: nil, + } + + err := apProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected Delete to fail but it succeeded") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Access Point is missing in volume Id", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + req := &csi.DeleteVolumeRequest{ + VolumeId: "fs-abcd1234", + } + + ctx := context.Background() + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: false, + mounter: nil, + } + + err := apProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected Delete to fail but it succeeded") + } + mockCtl.Finish() + }, + }, + { + name: "Fail: Cannot assume role for x-account", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + secrets := map[string]string{} + secrets["awsRoleArn"] = "arn:aws:iam::1234567890:role/EFSCrossAccountRole" + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + Secrets: secrets, + } + + ctx := context.Background() + + apProv := AccessPointProvisioner{ + tags: map[string]string{}, + cloud: mockCloud, + deleteAccessPointRootDir: true, + mounter: nil, + } + + err := apProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected Delete to fail but it succeeded") + } + mockCtl.Finish() + }, + }, + } + + for _, test := range tests { + t.Run(test.name, test.testFunc) + } +} diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 68b9077f0..5e049d9bb 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -19,12 +19,9 @@ package driver import ( "context" "fmt" - "os" - "strconv" "strings" "github.com/container-storage-interface/spec/lib/go/csi" - "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/cloud" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "k8s.io/klog/v2" @@ -39,6 +36,7 @@ const ( DefaultTagKey = "efs.csi.aws.com/cluster" DefaultTagValue = "true" DirectoryPerms = "directoryPerms" + DirectoryMode = "efs-dir" FsId = "fileSystemId" Gid = "gid" GidMin = "gidRangeStart" @@ -64,10 +62,6 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) return nil, status.Error(codes.InvalidArgument, "Volume name not provided") } - // Volume size is required to match PV to PVC by k8s. - // Volume size is not consumed by EFS for any purposes. - volSize := req.GetCapacityRange().GetRequiredBytes() - volCaps := req.GetVolumeCapabilities() if len(volCaps) == 0 { return nil, status.Error(codes.InvalidArgument, "Volume capabilities not provided") @@ -80,295 +74,63 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Volume fstype not supported: %s", err)) } - var ( - azName string - basePath string - err error - gid int - gidMin int - gidMax int - localCloud cloud.Cloud - provisioningMode string - roleArn string - uid int - ) - - //Parse parameters volumeParams := req.GetParameters() if value, ok := volumeParams[ProvisioningMode]; ok { - provisioningMode = value - //TODO: Add FS provisioning mode check when implemented - if provisioningMode != AccessPointMode { - errStr := "Provisioning mode " + provisioningMode + " is not supported. Only Access point provisioning: 'efs-ap' is supported" - return nil, status.Error(codes.InvalidArgument, errStr) + if _, ok = d.provisioners[value]; !ok { + return nil, status.Errorf(codes.InvalidArgument, "Provisioning mode %s is not supported.", value) } } else { return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", ProvisioningMode) } - // Create tags - tags := map[string]string{ - DefaultTagKey: DefaultTagValue, - } - - // Append input tags to default tag - if len(d.tags) != 0 { - for k, v := range d.tags { - tags[k] = v - } - } - - accessPointsOptions := &cloud.AccessPointOptions{ - CapacityGiB: volSize, - Tags: tags, - } - - if value, ok := volumeParams[FsId]; ok { - if strings.TrimSpace(value) == "" { - return nil, status.Errorf(codes.InvalidArgument, "Parameter %v cannot be empty", FsId) - } - accessPointsOptions.FileSystemId = value - } else { - return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", FsId) - } - - uid = -1 - if value, ok := volumeParams[Uid]; ok { - uid, err = strconv.Atoi(value) - if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", Uid, err) - } - if uid < 0 { - return nil, status.Errorf(codes.InvalidArgument, "%v must be greater or equal than 0", Uid) - } - } + mode := volumeParams[ProvisioningMode] + provisioner := d.provisioners[mode] + klog.V(5).Infof("CreateVolume: provisioning mode %s selected. Supported modes are %s", mode, + strings.Join(d.GetProvisioningModes(), ",")) - gid = -1 - if value, ok := volumeParams[Gid]; ok { - gid, err = strconv.Atoi(value) - if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", Gid, err) - } - if uid < 0 { - return nil, status.Errorf(codes.InvalidArgument, "%v must be greater or equal than 0", Gid) - } - } - - if value, ok := volumeParams[GidMin]; ok { - gidMin, err = strconv.Atoi(value) - if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", GidMin, err) - } - if gidMin <= 0 { - return nil, status.Errorf(codes.InvalidArgument, "%v must be greater than 0", GidMin) - } - } - - if value, ok := volumeParams[GidMax]; ok { - // Ensure GID min is provided with GID max - if gidMin == 0 { - return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", GidMin) - } - gidMax, err = strconv.Atoi(value) - if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", GidMax, err) - } - if gidMax <= gidMin { - return nil, status.Errorf(codes.InvalidArgument, "%v must be greater than %v", GidMax, GidMin) - } - } else { - // Ensure GID max is provided with GID min - if gidMin != 0 { - return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", GidMax) - } - } - - // Assign default GID ranges if not provided - if gidMin == 0 && gidMax == 0 { - gidMin = DefaultGidMin - gidMax = DefaultGidMax - } - - if value, ok := volumeParams[DirectoryPerms]; ok { - 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 - // The `az` mount option provided by efs-utils is used for cross az mount or to provide az of efs one zone file system mount within the same aws-account. - // To make use of the `az` mount option, add it under storage class's `mountOptions` section. https://kubernetes.io/docs/concepts/storage/storage-classes/#mount-options - if value, ok := volumeParams[AzName]; ok { - azName = value - } - - localCloud, roleArn, err = getCloud(req.GetSecrets(), d) + uid, gid, err := d.fsIdentityManager.GetUidAndGid( + volumeParams[Uid], volumeParams[Gid], volumeParams[GidMin], volumeParams[GidMax], volumeParams[FsId]) if err != nil { - return nil, err + return nil, status.Errorf(codes.Internal, "Could not find a free UID or GID") } + volume, err := provisioner.Provision(ctx, req, uid, gid) - // Check if file system exists. Describe FS handles appropriate error codes - if _, err = localCloud.DescribeFileSystem(ctx, accessPointsOptions.FileSystemId); err != nil { - 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.ErrNotFound { - return nil, status.Errorf(codes.InvalidArgument, "File System does not exist: %v", err) - } - return nil, status.Errorf(codes.Internal, "Failed to fetch File System info: %v", err) - } - - var allocatedGid int - if uid == -1 || gid == -1 { - allocatedGid, err = d.gidAllocator.getNextGid(accessPointsOptions.FileSystemId, gidMin, gidMax) - if err != nil { - return nil, err - } - } - if uid == -1 { - uid = allocatedGid - } - if gid == -1 { - gid = allocatedGid - } - - rootDirName := volName - rootDir := basePath + "/" + rootDirName - - accessPointsOptions.Uid = int64(uid) - accessPointsOptions.Gid = int64(gid) - accessPointsOptions.DirectoryPath = rootDir - - accessPointId, err := localCloud.CreateAccessPoint(ctx, volName, accessPointsOptions) if err != nil { - if allocatedGid != 0 { - 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) - } - - volContext := map[string]string{} - - // Fetch mount target Ip for cross-account mount - if roleArn != "" { - mountTarget, err := localCloud.DescribeMountTargets(ctx, accessPointsOptions.FileSystemId, azName) - if err != nil { - klog.Warningf("Failed to describe mount targets for file system %v. Skip using `mounttargetip` mount option: %v", accessPointsOptions.FileSystemId, err) - } else { - volContext[MountTargetIp] = mountTarget.IPAddress - } + d.fsIdentityManager.ReleaseGid(volumeParams[FsId], gid) + return nil, err } return &csi.CreateVolumeResponse{ - Volume: &csi.Volume{ - CapacityBytes: volSize, - VolumeId: accessPointsOptions.FileSystemId + "::" + accessPointId.AccessPointId, - VolumeContext: volContext, - }, + Volume: volume, }, nil } func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) { - var ( - localCloud cloud.Cloud - roleArn string - err error - ) - - localCloud, roleArn, err = getCloud(req.GetSecrets(), d) - if err != nil { - return nil, err - } - klog.V(4).Infof("DeleteVolume: called with args %+v", *req) volId := req.GetVolumeId() if volId == "" { return nil, status.Error(codes.InvalidArgument, "Volume ID not provided") } - fileSystemId, _, accessPointId, err := parseVolumeId(volId) + _, subpath, accessPointId, err := parseVolumeId(volId) if err != nil { //Returning success for an invalid volume ID. See here - https://github.com/kubernetes-csi/csi-test/blame/5deb83d58fea909b2895731d43e32400380aae3c/pkg/sanity/controller.go#L733 klog.V(5).Infof("DeleteVolume: Failed to parse volumeID: %v, err: %v, returning success", volId, err) return &csi.DeleteVolumeResponse{}, nil } - //TODO: Add Delete File System when FS provisioning is implemented if accessPointId != "" { - - // Delete access point root directory if delete-access-point-root-dir is set. - if d.deleteAccessPointRootDir { - // Check if Access point exists. - // If access point exists, retrieve its root directory and delete it/ - accessPoint, err := localCloud.DescribeAccessPoint(ctx, accessPointId) - if err != nil { - 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.ErrNotFound { - klog.V(5).Infof("DeleteVolume: Access Point %v not found, returning success", accessPointId) - return &csi.DeleteVolumeResponse{}, nil - } - return nil, status.Errorf(codes.Internal, "Could not get describe Access Point: %v , error: %v", accessPointId, err) - } - - //Mount File System at it root and delete access point root directory - mountOptions := []string{"tls", "iam"} - if roleArn != "" { - mountTarget, err := localCloud.DescribeMountTargets(ctx, fileSystemId, "") - - if err == nil { - mountOptions = append(mountOptions, MountTargetIp+"="+mountTarget.IPAddress) - } else { - klog.Warningf("Failed to describe mount targets for file system %v. Skip using `mounttargetip` mount option: %v", fileSystemId, err) - } - } - - target := TempMountPathPrefix + "/" + accessPointId - if err := d.mounter.MakeDir(target); err != nil { - return nil, status.Errorf(codes.Internal, "Could not create dir %q: %v", target, err) - } - if err := d.mounter.Mount(fileSystemId, target, "efs", mountOptions); err != nil { - os.Remove(target) - return nil, status.Errorf(codes.Internal, "Could not mount %q at %q: %v", fileSystemId, target, err) - } - err = os.RemoveAll(target + accessPoint.AccessPointRootDir) - if err != nil { - return nil, status.Errorf(codes.Internal, "Could not delete access point root directory %q: %v", accessPoint.AccessPointRootDir, err) - } - err = d.mounter.Unmount(target) - if err != nil { - return nil, status.Errorf(codes.Internal, "Could not unmount %q: %v", target, err) - } - err = os.RemoveAll(target) - if err != nil { - return nil, status.Errorf(codes.Internal, "Could not delete %q: %v", target, err) - } + err := d.provisioners[AccessPointMode].Delete(ctx, req) + if err != nil { + return nil, status.Errorf(codes.Internal, "Failed to Delete volume %v: %v", volId, err) } - - // Delete access point - if err = localCloud.DeleteAccessPoint(ctx, accessPointId); err != nil { - 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.ErrNotFound { - klog.V(5).Infof("DeleteVolume: Access Point not found, returning success") - return &csi.DeleteVolumeResponse{}, nil - } + } else if subpath != "" { + err := d.provisioners[DirectoryMode].Delete(ctx, req) + if err != nil { return nil, status.Errorf(codes.Internal, "Failed to Delete volume %v: %v", volId, err) } } else { - return nil, status.Errorf(codes.NotFound, "Failed to find access point for volume: %v", volId) + return nil, status.Errorf(codes.NotFound, "Failed to find identifying information for volume: %v", volId) } return &csi.DeleteVolumeResponse{}, nil @@ -452,27 +214,3 @@ func (d *Driver) ControllerGetVolume(ctx context.Context, req *csi.ControllerGet return nil, status.Error(codes.Unimplemented, "") } - -func getCloud(secrets map[string]string, driver *Driver) (cloud.Cloud, string, error) { - - var localCloud cloud.Cloud - var roleArn string - var err error - - // Fetch aws role ARN for cross account mount from CSI secrets. Link to CSI secrets below - // https://kubernetes-csi.github.io/docs/secrets-and-credentials.html#csi-operation-secrets - if value, ok := secrets[RoleArn]; ok { - roleArn = value - } - - if roleArn != "" { - localCloud, err = cloud.NewCloudWithRole(roleArn) - if err != nil { - return nil, "", status.Errorf(codes.Unauthenticated, "Unable to initialize aws cloud: %v. Please verify role has the correct AWS permissions for cross account mount", err) - } - } else { - localCloud = driver.cloud - } - - return localCloud, roleArn, nil -} diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index 151757b97..45865d6e5 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -3,23 +3,26 @@ package driver import ( "context" "errors" + "fmt" "testing" "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{ + endpoint = "endpoint" + volumeName = "volumeName" + fsId = "fs-abcd1234" + apId = "fsap-abcd1234xyz987" + apProvisioningVolumeId = "fs-abcd1234::fsap-abcd1234xyz987" + dirProvisioningVolumeId = "fs-abcd1234:/dynamic/" + volumeName + capacityRange int64 = 5368709120 + stdVolCap = &csi.VolumeCapability{ AccessType: &csi.VolumeCapability_Mount{ Mount: &csi.VolumeCapability_MountVolume{}, }, @@ -33,149 +36,12 @@ func TestCreateVolume(t *testing.T) { 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", + name: "Success: Normal flow, Access Point Provisioning", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - tags: parseTagsFromStr(""), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, @@ -216,23 +82,22 @@ func TestCreateVolume(t *testing.T) { t.Fatal("Volume is nil") } - if res.Volume.VolumeId != volumeId { - t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + if res.Volume.VolumeId != apProvisioningVolumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", apProvisioningVolumeId, res.Volume.VolumeId) } mockCtl.Finish() }, }, { - name: "Success: Using Default GID ranges", + name: "Success: Normal flow, Directory Provisioning", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(fsId, gomock.Any(), "efs", gomock.Any()).Return(nil) + mockMounter.EXPECT().Unmount(gomock.Any()).Return(nil) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, nil, "", mockMounter, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, @@ -243,23 +108,16 @@ func TestCreateVolume(t *testing.T) { RequiredBytes: capacityRange, }, Parameters: map[string]string{ - ProvisioningMode: "efs-ap", + ProvisioningMode: "efs-dir", FsId: fsId, + GidMin: "1000", + GidMax: "2000", DirectoryPerms: "777", - BasePath: "test", + BasePath: "/dynamic", }, } 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) @@ -271,24 +129,19 @@ func TestCreateVolume(t *testing.T) { t.Fatal("Volume is nil") } - if res.Volume.VolumeId != volumeId { - t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + if res.Volume.VolumeId != dirProvisioningVolumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", apProvisioningVolumeId, res.Volume.VolumeId) } mockCtl.Finish() }, }, { - name: "Success: Normal flow with tags", + name: "Success: Normal flow, no GID range specified", 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"), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, @@ -301,9 +154,8 @@ func TestCreateVolume(t *testing.T) { Parameters: map[string]string{ ProvisioningMode: "efs-ap", FsId: fsId, - GidMin: "1000", - GidMax: "2000", DirectoryPerms: "777", + BasePath: "test", }, } @@ -328,8 +180,8 @@ func TestCreateVolume(t *testing.T) { t.Fatal("Volume is nil") } - if res.Volume.VolumeId != volumeId { - t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + if res.Volume.VolumeId != apProvisioningVolumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", apProvisioningVolumeId, res.Volume.VolumeId) } mockCtl.Finish() }, @@ -340,12 +192,7 @@ func TestCreateVolume(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - tags: parseTagsFromStr("cluster-efs"), - } + driver := buildDriver(endpoint, mockCloud, "cluster-efs", nil, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, @@ -385,8 +232,8 @@ func TestCreateVolume(t *testing.T) { t.Fatal("Volume is nil") } - if res.Volume.VolumeId != volumeId { - t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + if res.Volume.VolumeId != apProvisioningVolumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", apProvisioningVolumeId, res.Volume.VolumeId) } mockCtl.Finish() }, @@ -397,11 +244,7 @@ func TestCreateVolume(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.CreateVolumeRequest{ Parameters: map[string]string{ @@ -425,11 +268,7 @@ func TestCreateVolume(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, @@ -454,11 +293,7 @@ func TestCreateVolume(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, @@ -486,11 +321,7 @@ func TestCreateVolume(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, @@ -528,12 +359,7 @@ func TestCreateVolume(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - tags: parseTagsFromStr(""), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, @@ -576,11 +402,7 @@ func TestCreateVolume(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, @@ -618,11 +440,7 @@ func TestCreateVolume(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, @@ -653,11 +471,7 @@ func TestCreateVolume(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, @@ -668,8 +482,9 @@ func TestCreateVolume(t *testing.T) { stdVolCap, }, Parameters: map[string]string{ - FsId: fsId, - DirectoryPerms: "777", + ProvisioningMode: "foobar", + FsId: fsId, + DirectoryPerms: "777", }, } @@ -682,1139 +497,169 @@ func TestCreateVolume(t *testing.T) { }, }, { - name: "Fail: Missing Parameter FsId", + name: "Fail: Run out of GIDs", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.CreateVolumeRequest{ Name: volumeName, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, VolumeCapabilities: []*csi.VolumeCapability{ stdVolCap, }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, Parameters: map[string]string{ ProvisioningMode: "efs-ap", + FsId: fsId, DirectoryPerms: "777", + GidMax: "1000", + GidMin: "1001", }, } ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + } + + mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil).AnyTimes() + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil).AnyTimes() + + var err error + // Input grants 2 GIDS, third CreateVolume call should result in error + for i := 0; i < 3; i++ { + _, err = driver.CreateVolume(ctx, req) + } + if err == nil { - t.Fatal("CreateVolume did not fail") + t.Fatalf("CreateVolume did not fail") } mockCtl.Finish() }, }, + } + + for _, tc := range testCases { + t.Run(tc.name, tc.testFunc) + } +} + +func TestDeleteVolume(t *testing.T) { + var ( + fsId = "fs-abcd1234" + apId = "fsap-abcd1234xyz987" + provisionedDir = "/dynamic/newVolume" + endpoint = "endpoint" + apProvisionedVolumeId = fmt.Sprintf("%s::%s", fsId, apId) + dirProvisionedVolumeId = fmt.Sprintf("%s:%s", fsId, provisionedDir) + ) + + testCases := []struct { + name string + testFunc func(t *testing.T) + }{ { - name: "Fail: FsId cannot be blank", + name: "Success: Normal flow, Access Point Provisioning", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) - req := &csi.CreateVolumeRequest{ - Name: volumeName, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, - VolumeCapabilities: []*csi.VolumeCapability{ - stdVolCap, - }, - Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: " ", - DirectoryPerms: "777", - }, + req := &csi.DeleteVolumeRequest{ + VolumeId: apProvisionedVolumeId, } ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") + mockCloud.EXPECT().DeleteAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(nil) + _, err := driver.DeleteVolume(ctx, req) + if err != nil { + t.Fatalf("Delete Volume failed: %v", err) } mockCtl.Finish() }, }, { - name: "Fail: Uid invalid", + name: "Success: Normal flow, Directory Provisioning", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(fsId, gomock.Any(), "efs", gomock.Any()).Return(nil) + mockMounter.EXPECT().Unmount(gomock.Any()).Return(nil) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, mockCloud, "", mockMounter, false, true) - req := &csi.CreateVolumeRequest{ - Name: volumeName, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, - VolumeCapabilities: []*csi.VolumeCapability{ - stdVolCap, - }, - Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - Uid: "invalid", - }, + req := &csi.DeleteVolumeRequest{ + VolumeId: dirProvisionedVolumeId, } ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") + _, err := driver.DeleteVolume(ctx, req) + if err != nil { + t.Fatalf("Delete Volume failed: %v", err) } mockCtl.Finish() }, }, { - name: "Fail: Uid cannot be negative", + name: "Success: Cannot parse details from volumeId", + testFunc: func(t *testing.T) { + driver := buildDriver(endpoint, nil, "", nil, false, true) + + req := &csi.DeleteVolumeRequest{ + VolumeId: "foobarbash", + } + + ctx := context.Background() + _, err := driver.DeleteVolume(ctx, req) + if err != nil { + t.Fatalf("Expected success but found: %v", err) + } + }, + }, + { + name: "Fail: DeleteVolume fails if access point cannot be deleted", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) + ctx := context.Background() + mockCloud.EXPECT().DeleteAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(errors.New("Delete Volume failed")) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) - req := &csi.CreateVolumeRequest{ - Name: volumeName, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, - VolumeCapabilities: []*csi.VolumeCapability{ - stdVolCap, - }, - Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - Uid: "-5", - }, + req := &csi.DeleteVolumeRequest{ + VolumeId: apProvisionedVolumeId, } - ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) + _, err := driver.DeleteVolume(ctx, req) if err == nil { - t.Fatal("CreateVolume did not fail") + t.Fatal("DeleteVolume did not fail") } mockCtl.Finish() }, }, { - name: "Fail: Gid invalid", + name: "Fail: volumeId not provided", testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) + driver := buildDriver(endpoint, nil, "", nil, false, true) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } - - req := &csi.CreateVolumeRequest{ - Name: volumeName, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, - VolumeCapabilities: []*csi.VolumeCapability{ - stdVolCap, - }, - Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - Gid: "invalid", - }, - } - - ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: Gid cannot be negative", - 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, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, - VolumeCapabilities: []*csi.VolumeCapability{ - stdVolCap, - }, - Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - Gid: "-5", - }, - } - - ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: Gid min cannot be 0", - 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, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, - VolumeCapabilities: []*csi.VolumeCapability{ - stdVolCap, - }, - Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - GidMin: "0", - }, - } - - ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: GidMin must be an integer", - 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, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, - VolumeCapabilities: []*csi.VolumeCapability{ - stdVolCap, - }, - Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - GidMin: "test", - }, - } - - ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: GidMax must be an integer", - 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, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, - VolumeCapabilities: []*csi.VolumeCapability{ - stdVolCap, - }, - Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - GidMin: "2000", - GidMax: "test", - }, - } - - ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: GidMax must be greater than GidMin", - 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, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, - VolumeCapabilities: []*csi.VolumeCapability{ - stdVolCap, - }, - Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - GidMin: "2000", - GidMax: "1000", - }, - } - - ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: GidMax must be provided with GidMin", - 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, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, - VolumeCapabilities: []*csi.VolumeCapability{ - stdVolCap, - }, - Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - GidMin: "2000", - }, - } - - ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: GidMin must be provided with GidMax", - 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, - CapacityRange: &csi.CapacityRange{ - RequiredBytes: capacityRange, - }, - VolumeCapabilities: []*csi.VolumeCapability{ - stdVolCap, - }, - Parameters: map[string]string{ - ProvisioningMode: "efs-ap", - FsId: fsId, - DirectoryPerms: "777", - GidMax: "2000", - }, - } - - ctx := context.Background() - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: File system does not exist", - 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, - GidMin: "1000", - GidMax: "2000", - DirectoryPerms: "777", - }, - } - - ctx := context.Background() - mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(nil, cloud.ErrNotFound) - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: DescribeFileSystem Access Denied", - 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, - GidMin: "1000", - GidMax: "2000", - DirectoryPerms: "777", - }, - } - - ctx := context.Background() - mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(nil, cloud.ErrAccessDenied) - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: Describe File system call fails", - 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, - GidMin: "1000", - GidMax: "2000", - DirectoryPerms: "777", - }, - } - - ctx := context.Background() - mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(nil, errors.New("DescribeFileSystem failed")) - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: Create Access Point call fails", - 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, - GidMin: "1000", - GidMax: "2000", - DirectoryPerms: "777", - }, - } - - ctx := context.Background() - fileSystem := &cloud.FileSystem{ - FileSystemId: fsId, - } - mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(nil, errors.New("CreateAccessPoint call failed")) - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: CreateAccessPoint Access Denied", - 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, - GidMin: "1000", - GidMax: "2000", - DirectoryPerms: "777", - }, - } - - ctx := context.Background() - fileSystem := &cloud.FileSystem{ - FileSystemId: fsId, - } - mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(nil, cloud.ErrAccessDenied) - _, err := driver.CreateVolume(ctx, req) - if err == nil { - t.Fatal("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: Run out of GIDs", - 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, - GidMin: "1000", - GidMax: "1001", - 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).AnyTimes() - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil).AnyTimes() - - var err error - // Input grants 2 GIDS, third CreateVolume call should result in error - for i := 0; i < 3; i++ { - _, err = driver.CreateVolume(ctx, req) - } - - if err == nil { - t.Fatalf("CreateVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: Cannot assume role for x-account", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - tags: parseTagsFromStr(""), - } - - secrets := map[string]string{} - secrets["awsRoleArn"] = "arn:aws:iam::1234567890:role/EFSCrossAccountRole" - - 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", - }, - Secrets: secrets, - } - - ctx := context.Background() - - _, err := driver.CreateVolume(ctx, req) - - if err == nil { - t.Fatalf("CreateVolume did not fail") - } - - mockCtl.Finish() - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, tc.testFunc) - } -} - -func TestDeleteVolume(t *testing.T) { - var ( - apId = "fsap-abcd1234xyz987" - fsId = "fs-abcd1234" - endpoint = "endpoint" - volumeId = "fs-abcd1234::fsap-abcd1234xyz987" - ) - - testCases := []struct { - name string - testFunc func(t *testing.T) - }{ - { - 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(), - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - } - - ctx := context.Background() - mockCloud.EXPECT().DeleteAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(nil) - _, err := driver.DeleteVolume(ctx, req) - if err != nil { - t.Fatalf("Delete Volume failed: %v", err) - } - mockCtl.Finish() - }, - }, - { - name: "Success: Normal flow with deleteAccessPointRootDir", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - mockMounter := mocks.NewMockMounter(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - mounter: mockMounter, - gidAllocator: NewGidAllocator(), - deleteAccessPointRootDir: true, - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - } - - accessPoint := &cloud.AccessPoint{ - AccessPointId: apId, - FileSystemId: fsId, - AccessPointRootDir: "", - CapacityGiB: 0, - } - - ctx := context.Background() - mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) - mockMounter.EXPECT().Mount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) - mockMounter.EXPECT().Unmount(gomock.Any()).Return(nil) - mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(accessPoint, nil) - mockCloud.EXPECT().DeleteAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(nil) - _, err := driver.DeleteVolume(ctx, req) - if err != nil { - t.Fatalf("Delete Volume failed: %v", err) - } - mockCtl.Finish() - }, - }, - { - name: "Success: DescribeAccessPoint Access Point Does not exist", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - mockMounter := mocks.NewMockMounter(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - mounter: mockMounter, - gidAllocator: NewGidAllocator(), - deleteAccessPointRootDir: true, - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - } - - ctx := context.Background() - mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(nil, cloud.ErrNotFound) - _, err := driver.DeleteVolume(ctx, req) - if err != nil { - t.Fatalf("Delete Volume failed: %v", err) - } - mockCtl.Finish() - }, - }, - { - name: "Fail: DescribeAccessPoint Access Denied", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - mockMounter := mocks.NewMockMounter(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - mounter: mockMounter, - gidAllocator: NewGidAllocator(), - deleteAccessPointRootDir: true, - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - } - - ctx := context.Background() - mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(nil, cloud.ErrAccessDenied) - _, err := driver.DeleteVolume(ctx, req) - if err == nil { - t.Fatalf("DeleteVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: DescribeAccessPoint failed", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - mockMounter := mocks.NewMockMounter(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - mounter: mockMounter, - gidAllocator: NewGidAllocator(), - deleteAccessPointRootDir: true, - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - } - - ctx := context.Background() - mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(nil, errors.New("Describe Access Point failed")) - _, err := driver.DeleteVolume(ctx, req) - if err == nil { - t.Fatalf("DeleteVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: Fail to make directory for access point mount", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - mockMounter := mocks.NewMockMounter(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - mounter: mockMounter, - gidAllocator: NewGidAllocator(), - deleteAccessPointRootDir: true, - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - } - - accessPoint := &cloud.AccessPoint{ - AccessPointId: apId, - FileSystemId: fsId, - AccessPointRootDir: "", - CapacityGiB: 0, - } - - ctx := context.Background() - mockMounter.EXPECT().MakeDir(gomock.Any()).Return(errors.New("Failed to makeDir")) - mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(accessPoint, nil) - _, err := driver.DeleteVolume(ctx, req) - if err == nil { - t.Fatal("DeleteVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: Fail to mount file system on directory for access point root directory removal", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - mockMounter := mocks.NewMockMounter(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - mounter: mockMounter, - gidAllocator: NewGidAllocator(), - deleteAccessPointRootDir: true, - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - } - - accessPoint := &cloud.AccessPoint{ - AccessPointId: apId, - FileSystemId: fsId, - AccessPointRootDir: "", - CapacityGiB: 0, - } - - ctx := context.Background() - mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) - mockMounter.EXPECT().Mount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("Failed to mount")) - mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(accessPoint, nil) - _, err := driver.DeleteVolume(ctx, req) - if err == nil { - t.Fatal("DeleteVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: Fail to unmount file system after access point root directory removal", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - mockMounter := mocks.NewMockMounter(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - mounter: mockMounter, - gidAllocator: NewGidAllocator(), - deleteAccessPointRootDir: true, - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - } - - accessPoint := &cloud.AccessPoint{ - AccessPointId: apId, - FileSystemId: fsId, - AccessPointRootDir: "", - CapacityGiB: 0, - } - - ctx := context.Background() - mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) - mockMounter.EXPECT().Mount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) - mockMounter.EXPECT().Unmount(gomock.Any()).Return(errors.New("Failed to unmount")) - mockCloud.EXPECT().DescribeAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(accessPoint, nil) - _, err := driver.DeleteVolume(ctx, req) - if err == nil { - t.Fatal("DeleteVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Success: Access Point already deleted", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - } - - ctx := context.Background() - mockCloud.EXPECT().DeleteAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(cloud.ErrNotFound) - _, err := driver.DeleteVolume(ctx, req) - if err != nil { - t.Fatalf("Delete Volume failed: %v", err) - } - mockCtl.Finish() - }, - }, - { - name: "Fail: DeleteAccessPoint access denied", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - } - - ctx := context.Background() - mockCloud.EXPECT().DeleteAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(cloud.ErrAccessDenied) - _, err := driver.DeleteVolume(ctx, req) - if err == nil { - t.Fatal("DeleteVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: DeleteVolume fails", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - } - - ctx := context.Background() - mockCloud.EXPECT().DeleteAccessPoint(gomock.Eq(ctx), gomock.Eq(apId)).Return(errors.New("Delete Volume failed")) - _, err := driver.DeleteVolume(ctx, req) - if err == nil { - t.Fatal("DeleteVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: Access Point is missing in volume Id", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - } - - req := &csi.DeleteVolumeRequest{ - VolumeId: "fs-abcd1234", - } - - ctx := context.Background() - _, err := driver.DeleteVolume(ctx, req) - if err == nil { - t.Fatal("DeleteVolume did not fail") - } - mockCtl.Finish() - }, - }, - { - name: "Fail: Cannot assume role for x-account", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockCloud := mocks.NewMockCloud(mockCtl) - - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - gidAllocator: NewGidAllocator(), - tags: parseTagsFromStr(""), - } - - secrets := map[string]string{} - secrets["awsRoleArn"] = "arn:aws:iam::1234567890:role/EFSCrossAccountRole" - - req := &csi.DeleteVolumeRequest{ - VolumeId: volumeId, - Secrets: secrets, - } + req := &csi.DeleteVolumeRequest{} ctx := context.Background() - _, err := driver.DeleteVolume(ctx, req) - if err == nil { - t.Fatalf("DeleteVolume did not fail") + t.Fatal("Expected failure but operation succeeded") } - - mockCtl.Finish() }, }, } @@ -1855,10 +700,7 @@ func TestValidateVolumeCapabilities(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.ValidateVolumeCapabilitiesRequest{ VolumeId: volumeId, @@ -1885,10 +727,7 @@ func TestValidateVolumeCapabilities(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.ValidateVolumeCapabilitiesRequest{ VolumeId: volumeId, @@ -1915,10 +754,7 @@ func TestValidateVolumeCapabilities(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.ValidateVolumeCapabilitiesRequest{ VolumeCapabilities: []*csi.VolumeCapability{ @@ -1940,10 +776,7 @@ func TestValidateVolumeCapabilities(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) req := &csi.ValidateVolumeCapabilitiesRequest{ VolumeId: volumeId, @@ -1969,10 +802,7 @@ func TestControllerGetCapabilities(t *testing.T) { mockCtl := gomock.NewController(t) mockCloud := mocks.NewMockCloud(mockCtl) - driver := &Driver{ - endpoint: endpoint, - cloud: mockCloud, - } + driver := buildDriver(endpoint, mockCloud, "", nil, false, false) ctx := context.Background() _, err := driver.ControllerGetCapabilities(ctx, &csi.ControllerGetCapabilitiesRequest{}) @@ -1980,3 +810,17 @@ func TestControllerGetCapabilities(t *testing.T) { t.Fatalf("ControllerGetCapabilities failed: %v", err) } } + +func buildDriver(endpoint string, cloud cloud.Cloud, tags string, mounter Mounter, deleteAccessPointRootDir bool, deleteProvisionedDir bool) *Driver { + parsedTags := parseTagsFromStr(tags) + + driver := &Driver{ + endpoint: endpoint, + cloud: cloud, + provisioners: getProvisioners(parsedTags, cloud, deleteAccessPointRootDir, mounter, &FakeOsClient{}, deleteProvisionedDir), + tags: parsedTags, + mounter: mounter, + fsIdentityManager: NewFileSystemIdentityManager(), + } + return driver +} diff --git a/pkg/driver/directory_provisioner.go b/pkg/driver/directory_provisioner.go new file mode 100644 index 000000000..29bca98a3 --- /dev/null +++ b/pkg/driver/directory_provisioner.go @@ -0,0 +1,163 @@ +package driver + +import ( + "context" + "os" + "path" + "strconv" + "strings" + + "github.com/container-storage-interface/spec/lib/go/csi" + "github.com/google/uuid" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "k8s.io/klog/v2" + + "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/cloud" +) + +type DirectoryProvisioner struct { + mounter Mounter + cloud cloud.Cloud + osClient OsClient + deleteProvisionedDir bool +} + +func (d DirectoryProvisioner) Provision(ctx context.Context, req *csi.CreateVolumeRequest, _, _ int) (*csi.Volume, error) { + var provisionedPath string + + var fileSystemId string + volumeParams := req.GetParameters() + if value, ok := volumeParams[FsId]; ok { + if strings.TrimSpace(value) == "" { + return nil, status.Errorf(codes.InvalidArgument, "Parameter %v cannot be empty", FsId) + } + fileSystemId = value + } else { + return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", FsId) + } + klog.V(5).Infof("Provisioning directory on FileSystem %s...", fileSystemId) + + localCloud, roleArn, err := getCloud(d.cloud, req.GetSecrets()) + if err != nil { + return nil, err + } + + mountOptions, err := getMountOptions(ctx, localCloud, fileSystemId, roleArn) + if err != nil { + return nil, err + } + target := TempMountPathPrefix + "/" + uuid.New().String() + if err := d.mounter.MakeDir(target); err != nil { + return nil, status.Errorf(codes.Internal, "Could not create dir %q: %v", target, err) + } + if err := d.mounter.Mount(fileSystemId, target, "efs", mountOptions); err != nil { + return nil, status.Errorf(codes.Internal, "Could not mount %q at %q: %v", fileSystemId, target, err) + } + // Extract the basePath + var basePath string + if value, ok := volumeParams[BasePath]; ok { + basePath = value + } + + rootDirName := req.Name + provisionedPath = path.Join(basePath, rootDirName) + + klog.V(5).Infof("Provisioning directory at path %s", provisionedPath) + + // Grab the required permissions + perms := os.FileMode(0777) + if value, ok := volumeParams[DirectoryPerms]; ok { + parsedPerms, err := strconv.ParseUint(value, 8, 32) + if err == nil { + perms = os.FileMode(parsedPerms) + } + } + + klog.V(5).Infof("Provisioning directory with permissions %s", perms) + + provisionedDirectory := path.Join(target, provisionedPath) + err = d.osClient.MkDirAllWithPermsNoOwnership(provisionedDirectory, perms) + if err != nil { + return nil, status.Errorf(codes.Internal, "Could not provision directory: %v", err) + } + + // Check the permissions that actually got created + actualPerms, err := d.osClient.GetPerms(provisionedDirectory) + if err != nil { + klog.V(5).Infof("Could not load file info for '%s'", provisionedDirectory) + } + klog.V(5).Infof("Permissions of folder '%s' are '%s'", provisionedDirectory, actualPerms) + + err = d.mounter.Unmount(target) + if err != nil { + return nil, status.Errorf(codes.Internal, "Could not unmount %q: %v", target, err) + } + err = d.osClient.RemoveAll(target) + if err != nil { + return nil, status.Errorf(codes.Internal, "Could not delete %q: %v", target, err) + } + + return &csi.Volume{ + CapacityBytes: req.GetCapacityRange().GetRequiredBytes(), + VolumeId: fileSystemId + ":" + provisionedPath, + VolumeContext: map[string]string{}, + }, nil +} + +func (d DirectoryProvisioner) Delete(ctx context.Context, req *csi.DeleteVolumeRequest) (e error) { + if !d.deleteProvisionedDir { + return nil + } + fileSystemId, subpath, _, _ := parseVolumeId(req.GetVolumeId()) + klog.V(5).Infof("Running delete for EFS %s at subpath %s", fileSystemId, subpath) + + localCloud, roleArn, err := getCloud(d.cloud, req.GetSecrets()) + if err != nil { + return err + } + + mountOptions, err := getMountOptions(ctx, localCloud, fileSystemId, roleArn) + if err != nil { + return err + } + + target := TempMountPathPrefix + "/" + uuid.New().String() + klog.V(5).Infof("Making temporary directory at '%s' to temporarily mount EFS folder in", target) + if err := d.mounter.MakeDir(target); err != nil { + return status.Errorf(codes.Internal, "Could not create dir %q: %v", target, err) + } + + defer func() { + // Try and unmount the directory + klog.V(5).Infof("Unmounting directory mounted at '%s'", target) + unmountErr := d.mounter.Unmount(target) + // If that fails then track the error but don't do anything else + if unmountErr != nil { + klog.V(5).Infof("Unmount failed at '%s'", target) + e = status.Errorf(codes.Internal, "Could not unmount %q: %v", target, err) + } else { + // If it is nil then it's safe to try and delete the directory as it should now be empty + klog.V(5).Infof("Deleting temporary directory at '%s'", target) + if err := d.osClient.RemoveAll(target); err != nil { + e = status.Errorf(codes.Internal, "Could not delete %q: %v", target, err) + } + } + }() + + klog.V(5).Infof("Mounting EFS '%s' into temporary directory at '%s'", fileSystemId, target) + if err := d.mounter.Mount(fileSystemId, target, "efs", mountOptions); err != nil { + // If this call throws an error we're about to return anyway and the mount has failed, so it's more + // important we return with that information than worry about the folder not being deleted + _ = d.osClient.Remove(target) + return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", fileSystemId, target, err) + } + + pathToRemove := path.Join(target, subpath) + klog.V(5).Infof("Delete all files at %s, stored on EFS %s", pathToRemove, fileSystemId) + if err := d.osClient.RemoveAll(pathToRemove); err != nil { + return status.Errorf(codes.Internal, "Could not delete directory %q: %v", subpath, err) + } + + return nil +} diff --git a/pkg/driver/directory_provisioner_test.go b/pkg/driver/directory_provisioner_test.go new file mode 100644 index 000000000..26fe50bfd --- /dev/null +++ b/pkg/driver/directory_provisioner_test.go @@ -0,0 +1,576 @@ +package driver + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "testing" + + "github.com/container-storage-interface/spec/lib/go/csi" + "github.com/golang/mock/gomock" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "k8s.io/mount-utils" + + "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/driver/mocks" +) + +func TestDirectoryProvisioner_Provision(t *testing.T) { + var ( + fsId = "fs-abcd1234" + volumeName = "volumeName" + 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, + }, + } + ) + tests := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Success: Check path created is sensible", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(fsId, gomock.Any(), "efs", gomock.Any()).Return(nil) + mockMounter.EXPECT().Unmount(gomock.Any()).Return(nil) + + ctx := context.Background() + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + Parameters: map[string]string{ + ProvisioningMode: DirectoryMode, + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + BasePath: "/dynamic", + }, + } + + dProv := DirectoryProvisioner{ + cloud: nil, + mounter: mockMounter, + osClient: &FakeOsClient{}, + } + + volume, err := dProv.Provision(ctx, req, 1000, 1000) + + if err != nil { + t.Fatalf("Expected provision call to succeed but failed: %v", err) + } + + expectedVolumeId := fmt.Sprintf("%s:/dynamic/%s", fsId, req.Name) + if volume.VolumeId != expectedVolumeId { + t.Fatalf("Expected volumeId to be %s but was %s", expectedVolumeId, volume.VolumeId) + } + }, + }, + { + name: "Fail: Return error for failed x-account mount", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + + ctx := context.Background() + + fakeRoleArn := "foo-bar" + req := &csi.CreateVolumeRequest{ + Name: volumeName, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + Parameters: map[string]string{ + ProvisioningMode: DirectoryMode, + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + }, + Secrets: map[string]string{ + RoleArn: fakeRoleArn, + }, + } + + dProv := DirectoryProvisioner{ + cloud: nil, + mounter: mockMounter, + } + + _, err := dProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected error but found none") + } + if status.Code(err) != codes.Unauthenticated { + t.Fatalf("Expected unauthenticated error but instead got %v", err) + } + }, + }, + { + name: "Fail: Return error for empty fsId", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + + ctx := context.Background() + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + Parameters: map[string]string{ + ProvisioningMode: DirectoryMode, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + }, + } + + dProv := DirectoryProvisioner{ + cloud: nil, + mounter: mockMounter, + } + + _, err := dProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected error but found none") + } + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("Expected InvalidArgument error but instead got %v", err) + } + }, + }, + { + name: "Fail: Mounter cannot create target directory on node", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return( + io.ErrUnexpectedEOF) + + ctx := context.Background() + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + Parameters: map[string]string{ + ProvisioningMode: DirectoryMode, + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + }, + } + + dProv := DirectoryProvisioner{ + cloud: nil, + mounter: mockMounter, + } + + _, err := dProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected error but found none") + } + if status.Code(err) != codes.Internal && errors.Is(errors.Unwrap(err), io.ErrUnexpectedEOF) { + t.Fatalf("Expected mount error but instead got %v", err) + } + }, + }, + { + name: "Fail: Mounter cannot mount into target directory", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(fsId, gomock.Any(), "efs", gomock.Any()).Return( + mount.NewMountError(mount.HasFilesystemErrors, "Errors")) + + ctx := context.Background() + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + Parameters: map[string]string{ + ProvisioningMode: DirectoryMode, + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + }, + } + + dProv := DirectoryProvisioner{ + cloud: nil, + mounter: mockMounter, + } + + _, err := dProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected error but found none") + } + if status.Code(err) != codes.Internal && errors.Is(errors.Unwrap(err), mount.MountError{}) { + t.Fatalf("Expected mount error but instead got %v", err) + } + }, + }, + { + name: "Fail: Could not create directory after mounting root", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(fsId, gomock.Any(), "efs", gomock.Any()).Return(nil) + + ctx := context.Background() + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + Parameters: map[string]string{ + ProvisioningMode: DirectoryMode, + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + BasePath: "/dynamic", + }, + } + + dProv := DirectoryProvisioner{ + cloud: nil, + mounter: mockMounter, + osClient: &BrokenOsClient{}, + } + + _, err := dProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected error but found none") + } + if status.Code(err) != codes.Internal && errors.Is(errors.Unwrap(err), &os.PathError{}) { + t.Fatalf("Expected path error but instead got %v", err) + } + }, + }, + { + name: "Fail: Could not unmount root directory post creation", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(fsId, gomock.Any(), "efs", gomock.Any()).Return(nil) + mockMounter.EXPECT().Unmount(gomock.Any()).Return(mount.NewMountError(mount.FilesystemMismatch, "Error")) + + ctx := context.Background() + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + Parameters: map[string]string{ + ProvisioningMode: DirectoryMode, + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + BasePath: "/dynamic", + }, + } + + dProv := DirectoryProvisioner{ + cloud: nil, + mounter: mockMounter, + osClient: &FakeOsClient{}, + } + + _, err := dProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected error but found none") + } + if status.Code(err) != codes.Internal && errors.Is(errors.Unwrap(err), mount.MountError{}) { + t.Fatalf("Expected mount error but instead got %v", err) + } + }, + }, + { + name: "Fail: Could not delete target directory once unmounted", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(fsId, gomock.Any(), "efs", gomock.Any()).Return(nil) + + ctx := context.Background() + + req := &csi.CreateVolumeRequest{ + Name: volumeName, + VolumeCapabilities: []*csi.VolumeCapability{ + stdVolCap, + }, + CapacityRange: &csi.CapacityRange{ + RequiredBytes: capacityRange, + }, + Parameters: map[string]string{ + ProvisioningMode: DirectoryMode, + FsId: fsId, + GidMin: "1000", + GidMax: "2000", + DirectoryPerms: "777", + BasePath: "/dynamic", + }, + } + + dProv := DirectoryProvisioner{ + cloud: nil, + mounter: mockMounter, + osClient: &BrokenOsClient{}, + } + + _, err := dProv.Provision(ctx, req, 1000, 1000) + + if err == nil { + t.Fatal("Expected error but found none") + } + if status.Code(err) != codes.Internal && errors.Is(errors.Unwrap(err), &os.PathError{}) { + t.Fatalf("Expected mount error but instead got %v", err) + } + }, + }, + } + for _, test := range tests { + t.Run(test.name, test.testFunc) + } +} + +func TestDirectoryProvisioner_Delete(t *testing.T) { + var ( + fsId = "fs-abcd1234" + volumeId = fmt.Sprintf("%s:%s", fsId, "/dynamic/newDir") + ) + + tests := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Success: If retain directory is set nothing happens", + testFunc: func(t *testing.T) { + ctx := context.Background() + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + dProv := DirectoryProvisioner{ + deleteProvisionedDir: false, + } + + err := dProv.Delete(ctx, req) + + if err != nil { + t.Fatalf("Expected success but found %v", err) + } + }, + }, + { + name: "Success: If not retaining directory folder and contents are deleted", + testFunc: func(t *testing.T) { + ctx := context.Background() + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(fsId, gomock.Any(), "efs", gomock.Any()).Return(nil) + mockMounter.EXPECT().Unmount(gomock.Any()).Return(nil) + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + dProv := DirectoryProvisioner{ + deleteProvisionedDir: true, + mounter: mockMounter, + osClient: &FakeOsClient{}, + } + + err := dProv.Delete(ctx, req) + + if err != nil { + t.Fatalf("Expected success but found %v", err) + } + }, + }, + { + name: "Fail: Mounter cannot create target directory on node", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return( + io.ErrUnexpectedEOF) + + ctx := context.Background() + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + dProv := DirectoryProvisioner{ + mounter: mockMounter, + deleteProvisionedDir: true, + } + + err := dProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected error but found none") + } + if status.Code(err) != codes.Internal && errors.Is(errors.Unwrap(err), io.ErrUnexpectedEOF) { + t.Fatalf("Expected mount error but instead got %v", err) + } + }, + }, + { + name: "Fail: Cannot delete contents of provisioned directory", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(fsId, gomock.Any(), "efs", gomock.Any()).Return(nil) + mockMounter.EXPECT().Unmount(gomock.Any()).Return(nil) + + ctx := context.Background() + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + dProv := DirectoryProvisioner{ + deleteProvisionedDir: true, + mounter: mockMounter, + osClient: &BrokenOsClient{}, + } + + err := dProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected error but found none") + } + if status.Code(err) != codes.Internal && errors.Is(errors.Unwrap(err), &os.PathError{}) { + t.Fatalf("Expected path error but instead got %v", err) + } + }, + }, + { + name: "Fail: Cannot unmount directory after contents have been deleted", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(fsId, gomock.Any(), "efs", gomock.Any()).Return(nil) + mockMounter.EXPECT().Unmount(gomock.Any()).Return(mount.NewMountError(mount.HasFilesystemErrors, "Errors")) + + ctx := context.Background() + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + dProv := DirectoryProvisioner{ + deleteProvisionedDir: true, + mounter: mockMounter, + osClient: &FakeOsClient{}, + } + + err := dProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected error but found none") + } + if status.Code(err) != codes.Internal && errors.Is(errors.Unwrap(err), mount.MountError{}) { + t.Fatalf("Expected mount error but instead got %v", err) + } + }, + }, + { + name: "Fail: Cannot delete temporary directory after unmount", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockMounter := mocks.NewMockMounter(mockCtl) + mockMounter.EXPECT().MakeDir(gomock.Any()).Return(nil) + mockMounter.EXPECT().Mount(fsId, gomock.Any(), "efs", gomock.Any()).Return(nil) + mockMounter.EXPECT().Unmount(gomock.Any()).Return(nil) + + ctx := context.Background() + + req := &csi.DeleteVolumeRequest{ + VolumeId: volumeId, + } + + dProv := DirectoryProvisioner{ + deleteProvisionedDir: true, + mounter: mockMounter, + osClient: &BrokenOsClient{}, + } + + err := dProv.Delete(ctx, req) + + if err == nil { + t.Fatal("Expected error but found none") + } + if status.Code(err) != codes.Internal && errors.Is(errors.Unwrap(err), &os.PathError{}) { + t.Fatalf("Expected path error but instead got %v", err) + } + }, + }, + } + + for _, test := range tests { + t.Run(test.name, test.testFunc) + } +} diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index c7a974b7d..965c10352 100644 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -39,18 +39,19 @@ type Driver struct { srv *grpc.Server mounter Mounter efsWatchdog Watchdog + provisioners map[string]Provisioner cloud cloud.Cloud nodeCaps []csi.NodeServiceCapability_RPC_Type volMetricsOptIn bool volMetricsRefreshPeriod float64 volMetricsFsRateLimit int volStatter VolStatter - gidAllocator GidAllocator + fsIdentityManager FileSystemIdentityManager deleteAccessPointRootDir bool tags map[string]string } -func NewDriver(endpoint, efsUtilsCfgPath, efsUtilsStaticFilesPath, tags string, volMetricsOptIn bool, volMetricsRefreshPeriod float64, volMetricsFsRateLimit int, deleteAccessPointRootDir bool) *Driver { +func NewDriver(endpoint, efsUtilsCfgPath, efsUtilsStaticFilesPath, tags string, volMetricsOptIn bool, volMetricsRefreshPeriod float64, volMetricsFsRateLimit int, deleteAccessPointRootDir bool, deleteProvisionedDir bool) *Driver { cloud, err := cloud.NewCloud() if err != nil { klog.Fatalln(err) @@ -58,32 +59,25 @@ func NewDriver(endpoint, efsUtilsCfgPath, efsUtilsStaticFilesPath, tags string, nodeCaps := SetNodeCapOptInFeatures(volMetricsOptIn) watchdog := newExecWatchdog(efsUtilsCfgPath, efsUtilsStaticFilesPath, "amazon-efs-mount-watchdog") - return &Driver{ - endpoint: endpoint, - nodeID: cloud.GetMetadata().GetInstanceID(), - mounter: newNodeMounter(), - efsWatchdog: watchdog, - cloud: cloud, - nodeCaps: nodeCaps, - volStatter: NewVolStatter(), - volMetricsOptIn: volMetricsOptIn, - volMetricsRefreshPeriod: volMetricsRefreshPeriod, - volMetricsFsRateLimit: volMetricsFsRateLimit, - gidAllocator: NewGidAllocator(), - deleteAccessPointRootDir: deleteAccessPointRootDir, - tags: parseTagsFromStr(strings.TrimSpace(tags)), - } -} + parsedTags := parseTagsFromStr(strings.TrimSpace(tags)) + mounter := newNodeMounter() + provisioners := getProvisioners(parsedTags, cloud, deleteAccessPointRootDir, mounter, &RealOsClient{}, deleteProvisionedDir) -func SetNodeCapOptInFeatures(volMetricsOptIn bool) []csi.NodeServiceCapability_RPC_Type { - var nCaps = []csi.NodeServiceCapability_RPC_Type{} - if volMetricsOptIn { - klog.V(4).Infof("Enabling Node Service capability for Get Volume Stats") - nCaps = append(nCaps, csi.NodeServiceCapability_RPC_GET_VOLUME_STATS) - } else { - klog.V(4).Infof("Node Service capability for Get Volume Stats Not enabled") + return &Driver{ + endpoint: endpoint, + nodeID: cloud.GetMetadata().GetInstanceID(), + mounter: mounter, + efsWatchdog: watchdog, + provisioners: provisioners, + cloud: cloud, + nodeCaps: nodeCaps, + volStatter: NewVolStatter(), + volMetricsOptIn: volMetricsOptIn, + volMetricsRefreshPeriod: volMetricsRefreshPeriod, + volMetricsFsRateLimit: volMetricsFsRateLimit, + tags: parsedTags, + fsIdentityManager: NewFileSystemIdentityManager(), } - return nCaps } func (d *Driver) Run() error { @@ -128,6 +122,25 @@ func (d *Driver) Run() error { return d.srv.Serve(listener) } +func (d *Driver) GetProvisioningModes() []string { + var keys []string + for k := range d.provisioners { + keys = append(keys, k) + } + return keys +} + +func SetNodeCapOptInFeatures(volMetricsOptIn bool) []csi.NodeServiceCapability_RPC_Type { + var nCaps = []csi.NodeServiceCapability_RPC_Type{} + if volMetricsOptIn { + klog.V(4).Infof("Enabling Node Service capability for Get Volume Stats") + nCaps = append(nCaps, csi.NodeServiceCapability_RPC_GET_VOLUME_STATS) + } else { + klog.V(4).Infof("Node Service capability for Get Volume Stats Not enabled") + } + return nCaps +} + func parseTagsFromStr(tagStr string) map[string]string { defer func() { if r := recover(); r != nil { diff --git a/pkg/driver/fs_identifier_manager.go b/pkg/driver/fs_identifier_manager.go new file mode 100644 index 000000000..f4ad469a3 --- /dev/null +++ b/pkg/driver/fs_identifier_manager.go @@ -0,0 +1,190 @@ +package driver + +import ( + "container/heap" + "strconv" + "sync" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "k8s.io/klog/v2" +) + +type FileSystemIdentityManager struct { + gidAllocator GidAllocator +} + +func NewFileSystemIdentityManager() FileSystemIdentityManager { + return FileSystemIdentityManager{NewGidAllocator()} +} + +func (f *FileSystemIdentityManager) GetUidAndGid(rawUid string, rawGid string, rawGidMin string, rawGidMax string, + fsId string) (int, int, error) { + + var ( + uid int + gid int + err error + ) + + if rawGid != "" { + gid, err = f.extractId(rawGid) + if err != nil { + return -1, -1, err + } + } else { + gidMin, gidMax, err := f.parseGidMinAndMax(rawGidMin, rawGidMax) + if err != nil { + return -1, -1, err + } + allocatedGid, err := f.gidAllocator.getNextGid(fsId, int(gidMin), int(gidMax)) + if err != nil { + return -1, -1, err + } + gid = allocatedGid + } + + if rawUid == "" { + return gid, gid, nil + } else { + uid, err = f.extractId(rawUid) + if err != nil { + f.ReleaseGid(fsId, gid) + return -1, -1, err + } + } + + return uid, gid, nil +} + +func (f *FileSystemIdentityManager) ReleaseGid(fsId string, gid int) { + if gid != 0 { + f.gidAllocator.releaseGid(fsId, gid) + } +} + +func (f *FileSystemIdentityManager) parseGidMinAndMax(rawGidMin string, rawGidMax string) (int, int, error) { + if rawGidMin == "0" { + return -1, -1, status.Errorf(codes.InvalidArgument, "GidMin should be a > 0") + } + + if rawGidMin == "" && rawGidMax == "" { + return DefaultGidMin, DefaultGidMax, nil + } + + gidMin, err := f.extractId(rawGidMin) + if err != nil { + return -1, -1, err + } + gidMax, err := f.extractId(rawGidMax) + if err != nil { + return -1, -1, err + } + + if gidMin >= gidMax { + return -1, -1, status.Errorf(codes.InvalidArgument, "GidMin cannot be greater than GidMax") + } else if gidMin > 0 && gidMax == 0 { + return -1, -1, status.Errorf(codes.InvalidArgument, "Both GidMin and GidMax must be provided") + } + return gidMin, gidMax, nil +} + +func (f *FileSystemIdentityManager) extractId(rawId string) (int, error) { + id, err := strconv.ParseInt(rawId, 10, 32) + if err != nil { + return -1, err + } + if id < 0 { + return -1, status.Errorf(codes.InvalidArgument, "UID should be a positive integer but was %d", id) + } + return int(id), nil +} + +type GidAllocator struct { + fsIdGidMap map[string]*IntHeap + mu sync.Mutex +} + +func NewGidAllocator() GidAllocator { + return GidAllocator{ + fsIdGidMap: make(map[string]*IntHeap), + } +} + +// Retrieves the next available GID +func (g *GidAllocator) getNextGid(fsId string, gidMin, gidMax int) (int, error) { + g.mu.Lock() + defer g.mu.Unlock() + + klog.V(5).Infof("Recieved getNextGid for fsId: %v, min: %v, max: %v", fsId, gidMin, gidMax) + + if _, ok := g.fsIdGidMap[fsId]; !ok { + klog.V(5).Infof("FS Id doesn't exist, initializing...") + g.initFsId(fsId, gidMin, gidMax) + } + + gidHeap := g.fsIdGidMap[fsId] + + if gidHeap.Len() > 0 { + return heap.Pop(gidHeap).(int), nil + } else { + return 0, status.Errorf(codes.Internal, "Failed to locate a free GID for given the file system: %v. "+ + "Please create a new storage class with a new file-system", fsId) + } +} + +func (g *GidAllocator) releaseGid(fsId string, gid int) { + g.mu.Lock() + defer g.mu.Unlock() + + gidHeap := g.fsIdGidMap[fsId] + gidHeap.Push(gid) +} + +// Creates an entry fsIdGidMap if fsId does not exist. +func (g *GidAllocator) initFsId(fsId string, gidMin, gidMax int) { + h := initHeap(gidMin, gidMax) + heap.Init(h) + g.fsIdGidMap[fsId] = h +} + +func (g *GidAllocator) removeFsId(fsId string) { + g.mu.Lock() + defer g.mu.Unlock() + delete(g.fsIdGidMap, fsId) +} + +type IntHeap []int + +func (h IntHeap) Len() int { + return len(h) +} +func (h IntHeap) Less(i, j int) bool { + return h[i] < h[j] +} +func (h IntHeap) Swap(i, j int) { + h[i], h[j] = h[j], h[i] +} + +func (h *IntHeap) Push(x interface{}) { + *h = append(*h, x.(int)) +} + +func (h *IntHeap) Pop() interface{} { + old := *h + n := len(old) + x := old[n-1] + *h = old[0 : n-1] + return x +} + +// Initializes a heap inclusive of min & max +func initHeap(min, max int) *IntHeap { + h := make(IntHeap, max-min+1) + val := min + for i := range h { + h[i] = val + val += 1 + } + return &h +} diff --git a/pkg/driver/fs_identifier_manager_test.go b/pkg/driver/fs_identifier_manager_test.go new file mode 100644 index 000000000..a493e9472 --- /dev/null +++ b/pkg/driver/fs_identifier_manager_test.go @@ -0,0 +1,190 @@ +package driver + +import "testing" + +const fileSystemId = "fs-123456789" + +func TestGetUidAndGid(t *testing.T) { + tests := []struct { + name string + rawUid string + rawGid string + rawGidMin string + rawGidMax string + resultUid int + resultGid int + expectError bool + }{ + { + name: "Fixed UID and GID", + rawUid: "1000", + rawGid: "1001", + rawGidMin: "", + rawGidMax: "", + resultUid: 1000, + resultGid: 1001, + expectError: false, + }, + { + name: "Ranges are ignored if fixed UID and GID are specified", + rawUid: "1000", + rawGid: "1001", + rawGidMin: "5000", + rawGidMax: "70000", + resultUid: 1000, + resultGid: 1001, + expectError: false, + }, + { + name: "Invalid UID throws error", + rawUid: "invalid", + rawGid: "", + rawGidMin: "", + rawGidMax: "", + resultUid: -1, + resultGid: -1, + expectError: true, + }, + { + name: "Negative UID throws error", + rawUid: "-200", + rawGid: "", + rawGidMin: "", + rawGidMax: "", + resultUid: -1, + resultGid: -1, + expectError: true, + }, + { + name: "Invalid GID throws error", + rawUid: "", + rawGid: "invalid", + rawGidMin: "", + rawGidMax: "", + resultUid: -1, + resultGid: -1, + expectError: true, + }, + { + name: "Invalid GID throws error even if range is set", + rawUid: "", + rawGid: "invalid", + rawGidMin: "5000", + rawGidMax: "70000", + resultUid: -1, + resultGid: -1, + expectError: true, + }, + { + name: "Negative GID throws error", + rawUid: "", + rawGid: "-200", + rawGidMin: "", + rawGidMax: "", + resultUid: -1, + resultGid: -1, + expectError: true, + }, + { + name: "GID Range Used when Fixed GID not provided", + rawUid: "2001", + rawGid: "", + rawGidMin: "5000", + rawGidMax: "50000", + resultUid: 2001, + resultGid: 5000, + expectError: false, + }, + { + name: "GID Min cannot be 0", + rawUid: "2001", + rawGid: "", + rawGidMin: "0", + rawGidMax: "50000", + resultUid: -1, + resultGid: -1, + expectError: true, + }, + { + name: "GID Min must be numeric", + rawUid: "2001", + rawGid: "", + rawGidMin: "foo", + rawGidMax: "50000", + resultUid: -1, + resultGid: -1, + expectError: true, + }, + { + name: "GID Max must be numeric", + rawUid: "2001", + rawGid: "", + rawGidMin: "1000", + rawGidMax: "foo", + resultUid: -1, + resultGid: -1, + expectError: true, + }, + { + name: "GID Min must be less than GID Max", + rawUid: "2001", + rawGid: "", + rawGidMin: "500", + rawGidMax: "100", + resultUid: -1, + resultGid: -1, + expectError: true, + }, + { + name: "Both GID Min and GID Max must be provided", + rawUid: "2001", + rawGid: "", + rawGidMin: "500", + rawGidMax: "", + resultUid: -1, + resultGid: -1, + expectError: true, + }, + { + name: "If no GID parameters are provided fallback to the defaults", + rawUid: "2001", + rawGid: "", + rawGidMin: "", + rawGidMax: "", + resultUid: 2001, + resultGid: DefaultGidMin, + expectError: false, + }, + { + name: "If no UID/GID parameters are provided fallback to the defaults in both cases", + rawUid: "", + rawGid: "", + rawGidMin: "", + rawGidMax: "", + resultUid: DefaultGidMin, + resultGid: DefaultGidMin, + expectError: false, + }, + } + for _, test := range tests { + fsIdManager := NewFileSystemIdentityManager() + t.Run(test.name, func(t *testing.T) { + uid, gid, err := fsIdManager.GetUidAndGid(test.rawUid, test.rawGid, test.rawGidMin, test.rawGidMax, fileSystemId) + if test.expectError { + if err == nil { + t.Fatalf("Expected error but completed successfully") + } + } else { + if err != nil { + t.Fatalf("Didn't expect error but found %v", err) + } + } + if uid != test.resultUid { + t.Fatalf("Expected UID to be %d, but was %d", test.resultUid, uid) + } + if gid != test.resultGid { + t.Fatalf("Expected GID to be %d, but was %d", test.resultGid, gid) + } + }) + } +} diff --git a/pkg/driver/gid_allocator.go b/pkg/driver/gid_allocator.go deleted file mode 100644 index f672cdf08..000000000 --- a/pkg/driver/gid_allocator.go +++ /dev/null @@ -1,99 +0,0 @@ -package driver - -import ( - "container/heap" - "sync" - - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - "k8s.io/klog/v2" -) - -type IntHeap []int - -func (h IntHeap) Len() int { - return len(h) -} -func (h IntHeap) Less(i, j int) bool { - return h[i] < h[j] -} -func (h IntHeap) Swap(i, j int) { - h[i], h[j] = h[j], h[i] -} - -func (h *IntHeap) Push(x interface{}) { - *h = append(*h, x.(int)) -} - -func (h *IntHeap) Pop() interface{} { - old := *h - n := len(old) - x := old[n-1] - *h = old[0 : n-1] - return x -} - -type GidAllocator struct { - fsIdGidMap map[string]*IntHeap - mu sync.Mutex -} - -func NewGidAllocator() GidAllocator { - return GidAllocator{ - fsIdGidMap: make(map[string]*IntHeap), - } -} - -// Retrieves the next available GID -func (g *GidAllocator) getNextGid(fsId string, gidMin, gidMax int) (int, error) { - g.mu.Lock() - defer g.mu.Unlock() - - klog.V(5).Infof("Recieved getNextGid for fsId: %v, min: %v, max: %v", fsId, gidMin, gidMax) - - if _, ok := g.fsIdGidMap[fsId]; !ok { - klog.V(5).Infof("FS Id doesn't exist, initializing...") - g.initFsId(fsId, gidMin, gidMax) - } - - gidHeap := g.fsIdGidMap[fsId] - - if gidHeap.Len() > 0 { - return heap.Pop(gidHeap).(int), nil - } else { - return 0, status.Errorf(codes.Internal, "Failed to locate a free GID for given the file system: %v. "+ - "Please create a new storage class with a new file-system", fsId) - } -} - -func (g *GidAllocator) releaseGid(fsId string, gid int) { - g.mu.Lock() - defer g.mu.Unlock() - - gidHeap := g.fsIdGidMap[fsId] - gidHeap.Push(gid) -} - -// Creates an entry fsIdGidMap if fsId does not exist. -func (g *GidAllocator) initFsId(fsId string, gidMin, gidMax int) { - h := initHeap(gidMin, gidMax) - heap.Init(h) - g.fsIdGidMap[fsId] = h -} - -func (g *GidAllocator) removeFsId(fsId string) { - g.mu.Lock() - defer g.mu.Unlock() - delete(g.fsIdGidMap, fsId) -} - -// Initializes a heap inclusive of min & max -func initHeap(min, max int) *IntHeap { - h := make(IntHeap, max-min+1) - val := min - for i := range h { - h[i] = val - val += 1 - } - return &h -} diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index 3ef2f2daa..8bbeef93c 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -27,6 +27,7 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi" "github.com/golang/mock/gomock" + "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/driver/mocks" ) @@ -44,12 +45,13 @@ func setup(mockCtrl *gomock.Controller, volStatter VolStatter, volMetricsOptIn b mockMounter := mocks.NewMockMounter(mockCtrl) nodeCaps := SetNodeCapOptInFeatures(volMetricsOptIn) driver := &Driver{ - endpoint: "endpoint", - nodeID: "nodeID", - mounter: mockMounter, - volStatter: volStatter, - volMetricsOptIn: true, - nodeCaps: nodeCaps, + endpoint: "endpoint", + nodeID: "nodeID", + mounter: mockMounter, + volStatter: volStatter, + volMetricsOptIn: true, + nodeCaps: nodeCaps, + fsIdentityManager: NewFileSystemIdentityManager(), } ctx := context.Background() return mockMounter, driver, ctx diff --git a/pkg/driver/os_client.go b/pkg/driver/os_client.go new file mode 100644 index 000000000..59a2cf1ec --- /dev/null +++ b/pkg/driver/os_client.go @@ -0,0 +1,103 @@ +package driver + +import "os" + +type OsClient interface { + MkDirAllWithPerms(path string, perms os.FileMode, uid, gid int) error + MkDirAllWithPermsNoOwnership(path string, perms os.FileMode) error + GetPerms(path string) (os.FileMode, error) + Remove(path string) error + RemoveAll(path string) error +} + +type FakeOsClient struct{} + +func (o *FakeOsClient) MkDirAllWithPerms(_ string, _ os.FileMode, _, _ int) error { + return nil +} + +func (o *FakeOsClient) MkDirAllWithPermsNoOwnership(_ string, _ os.FileMode) error { + return nil +} + +func (o *FakeOsClient) Remove(_ string) error { + return nil +} + +func (o *FakeOsClient) RemoveAll(_ string) error { + return nil +} + +func (o *FakeOsClient) GetPerms(_ string) (os.FileMode, error) { + return 0, nil +} + +type BrokenOsClient struct{} + +func (o *BrokenOsClient) MkDirAllWithPerms(_ string, _ os.FileMode, _, _ int) error { + return &os.PathError{} +} + +func (o *BrokenOsClient) MkDirAllWithPermsNoOwnership(_ string, _ os.FileMode) error { + return &os.PathError{} +} + +func (o *BrokenOsClient) Remove(_ string) error { + return &os.PathError{} +} + +func (o *BrokenOsClient) RemoveAll(_ string) error { + return &os.PathError{} +} + +func (o *BrokenOsClient) GetPerms(path string) (os.FileMode, error) { + return 0, &os.PathError{} +} + +type RealOsClient struct{} + +func (o *RealOsClient) MkDirAllWithPerms(path string, perms os.FileMode, uid, gid int) error { + err := os.MkdirAll(path, perms) + if err != nil { + return err + } + // Extra CHMOD guarantees we get the permissions we desire, inspite of the umask + err = os.Chmod(path, perms) + if err != nil { + return err + } + err = os.Chown(path, uid, gid) + if err != nil { + return err + } + return nil +} + +func (o *RealOsClient) MkDirAllWithPermsNoOwnership(path string, perms os.FileMode) error { + err := os.MkdirAll(path, perms) + if err != nil { + return err + } + // Extra CHMOD guarantees we get the permissions we desire, inspite of the umask + err = os.Chmod(path, perms) + if err != nil { + return err + } + return nil +} + +func (o *RealOsClient) Remove(path string) error { + return os.Remove(path) +} + +func (o *RealOsClient) RemoveAll(path string) error { + return os.RemoveAll(path) +} + +func (o *RealOsClient) GetPerms(path string) (os.FileMode, error) { + fInfo, err := os.Stat(path) + if err != nil { + return 0, err + } + return fInfo.Mode(), nil +} diff --git a/pkg/driver/provisioner.go b/pkg/driver/provisioner.go new file mode 100644 index 000000000..912ba1b24 --- /dev/null +++ b/pkg/driver/provisioner.go @@ -0,0 +1,73 @@ +package driver + +import ( + "context" + + "github.com/container-storage-interface/spec/lib/go/csi" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "k8s.io/klog/v2" + + "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/cloud" +) + +type Provisioner interface { + Provision(ctx context.Context, req *csi.CreateVolumeRequest, uid, gid int) (*csi.Volume, error) + Delete(ctx context.Context, req *csi.DeleteVolumeRequest) error +} + +func getProvisioners(tags map[string]string, cloud cloud.Cloud, deleteAccessPointRootDir bool, mounter Mounter, osClient OsClient, deleteProvisionedDir bool) map[string]Provisioner { + return map[string]Provisioner{ + AccessPointMode: AccessPointProvisioner{ + tags: tags, + cloud: cloud, + deleteAccessPointRootDir: deleteAccessPointRootDir, + mounter: mounter, + }, + DirectoryMode: DirectoryProvisioner{ + mounter: mounter, + cloud: cloud, + osClient: osClient, + deleteProvisionedDir: deleteProvisionedDir, + }, + } +} + +func getCloud(originalCloud cloud.Cloud, secrets map[string]string) (cloud.Cloud, string, error) { + + var localCloud cloud.Cloud + var roleArn string + var err error + + // Fetch aws role ARN for cross account mount from CSI secrets. Link to CSI secrets below + // https://kubernetes-csi.github.io/docs/secrets-and-credentials.html#csi-operation-secrets + if value, ok := secrets[RoleArn]; ok { + roleArn = value + } + + if roleArn != "" { + localCloud, err = cloud.NewCloudWithRole(roleArn) + if err != nil { + return nil, "", status.Errorf(codes.Unauthenticated, "Unable to initialize aws cloud: %v. Please verify role has the correct AWS permissions for cross account mount", err) + } + } else { + localCloud = originalCloud + } + + return localCloud, roleArn, nil +} + +func getMountOptions(ctx context.Context, cloud cloud.Cloud, fileSystemId string, roleArn string) ([]string, error) { + //Mount File System at it root and delete access point root directory + mountOptions := []string{"tls", "iam"} + if roleArn != "" { + mountTarget, err := cloud.DescribeMountTargets(ctx, fileSystemId, "") + + if err == nil { + mountOptions = append(mountOptions, MountTargetIp+"="+mountTarget.IPAddress) + } else { + klog.Warningf("Failed to describe mount targets for file system %v. Skip using `mounttargetip` mount option: %v", fileSystemId, err) + } + } + return mountOptions, nil +} diff --git a/pkg/driver/provisioner_test.go b/pkg/driver/provisioner_test.go new file mode 100644 index 000000000..28ca93b8d --- /dev/null +++ b/pkg/driver/provisioner_test.go @@ -0,0 +1,77 @@ +package driver + +import ( + "context" + "reflect" + "testing" + + "github.com/golang/mock/gomock" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/cloud" + "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/driver/mocks" +) + +func TestProvisioner_GetCloud_NoRoleArnGivesOriginalObjectBack(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + actualCloud, _, _ := getCloud(mockCloud, map[string]string{}) + if actualCloud != mockCloud { + t.Fatalf("Expected cloud object to be %v but was %v", mockCloud, actualCloud) + } + + mockCtl.Finish() +} + +func TestProvisioner_GetCloud_IncorrectRoleArnGivesError(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + _, _, err := getCloud(mockCloud, map[string]string{ + RoleArn: "foo", + }) + if err == nil { + t.Fatalf("Expected error but none was returned") + } + if status.Code(err) != codes.Unauthenticated { + t.Fatalf("Expected 'Unauthenticated' error but found %v", err) + } + + mockCtl.Finish() +} + +func TestProvisioner_GetMountOptions_NoRoleArnGivesStandardOptions(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + ctx := context.Background() + expectedOptions := []string{"tls", "iam"} + + options, _ := getMountOptions(ctx, mockCloud, fileSystemId, "") + + if !reflect.DeepEqual(options, expectedOptions) { + t.Fatalf("Expected returned options to be %v but was %v", expectedOptions, options) + } +} + +func TestProvisioner_GetMountOptions_RoleArnAddsMountTargetIp(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + fakeMountTarget := cloud.MountTarget{ + AZName: "foo", + AZId: "", + MountTargetId: "", + IPAddress: "8.8.8.8", + } + ctx := context.Background() + mockCloud.EXPECT().DescribeMountTargets(ctx, fileSystemId, "").Return(&fakeMountTarget, nil) + + expectedOptions := []string{"tls", "iam", MountTargetIp + "=" + fakeMountTarget.IPAddress} + + options, _ := getMountOptions(ctx, mockCloud, fileSystemId, "roleArn") + + if !reflect.DeepEqual(options, expectedOptions) { + t.Fatalf("Expected returned options to be %v but was %v", expectedOptions, options) + } +} diff --git a/pkg/driver/sanity_test.go b/pkg/driver/sanity_test.go index 09d9770ca..2418fbf39 100644 --- a/pkg/driver/sanity_test.go +++ b/pkg/driver/sanity_test.go @@ -68,16 +68,20 @@ func TestSanityEFSCSI(t *testing.T) { nodeCaps := SetNodeCapOptInFeatures(true) mockCtrl := gomock.NewController(t) + mockCloud := cloud.NewFakeCloudProvider() + mounter := NewFakeMounter() + drv := Driver{ - endpoint: endpoint, - nodeID: "sanity", - mounter: NewFakeMounter(), - efsWatchdog: &mockWatchdog{}, - cloud: cloud.NewFakeCloudProvider(), - nodeCaps: nodeCaps, - volMetricsOptIn: true, - volStatter: NewVolStatter(), - gidAllocator: NewGidAllocator(), + endpoint: endpoint, + nodeID: "sanity", + mounter: mounter, + efsWatchdog: &mockWatchdog{}, + cloud: mockCloud, + nodeCaps: nodeCaps, + volMetricsOptIn: true, + volStatter: NewVolStatter(), + provisioners: getProvisioners(nil, mockCloud, false, mounter, &FakeOsClient{}, false), + fsIdentityManager: NewFileSystemIdentityManager(), } defer func() { if r := recover(); r != nil { diff --git a/test/e2e/e2e.go b/test/e2e/e2e.go index ac228a7a9..603f736ec 100644 --- a/test/e2e/e2e.go +++ b/test/e2e/e2e.go @@ -3,6 +3,7 @@ package e2e import ( "context" "fmt" + e2epv "k8s.io/kubernetes/test/e2e/framework/pv" "os" "strconv" "strings" @@ -360,6 +361,98 @@ var _ = ginkgo.Describe("[efs-csi] EFS CSI", func() { encryptInTransit := false testEncryptInTransit(f, &encryptInTransit) }) + + createProvisionedDirectory := func(f *framework.Framework, basePath string, pvcName string) (*v1.PersistentVolumeClaim, *storagev1.StorageClass) { + immediateBinding := storagev1.VolumeBindingImmediate + sc := storageframework.GetStorageClass("efs.csi.aws.com", map[string]string{ + "provisioningMode": "efs-dir", + "fileSystemId": FileSystemId, + "basePath": basePath, + }, &immediateBinding, f.Namespace.Name) + sc, err := f.ClientSet.StorageV1().StorageClasses().Create(context.TODO(), sc, metav1.CreateOptions{}) + framework.Logf("Created StorageClass %s", sc.Name) + framework.ExpectNoError(err, "creating dynamic provisioning storage class") + pvc := makeEFSPVC(f.Namespace.Name, pvcName, sc.Name) + pvc, err = f.ClientSet.CoreV1().PersistentVolumeClaims(f.Namespace.Name).Create(context.TODO(), pvc, metav1.CreateOptions{}) + err = e2epv.WaitForPersistentVolumeClaimPhase(v1.ClaimBound, f.ClientSet, f.Namespace.Name, pvc.Name, + time.Second*5, time.Minute) + framework.ExpectNoError(err, "waiting for pv to be provisioned and bound") + pvc, err = f.ClientSet.CoreV1().PersistentVolumeClaims(f.Namespace.Name).Get(context.TODO(), pvc.Name, metav1.GetOptions{}) + + framework.Logf("Created PVC %s, bound to PV %s by dynamic provisioning", sc.Name, pvc.Name, pvc.Spec.VolumeName) + return pvc, sc + } + + ginkgo.It("should create a directory with the correct permissions when in directory provisioning mode", func() { + basePath := "dynamic_provisioning" + dynamicPvc, sc := createProvisionedDirectory(f, basePath, "directory-pvc-1") + defer func() { + err := f.ClientSet.StorageV1().StorageClasses().Delete(context.TODO(), sc.Name, metav1.DeleteOptions{}) + framework.ExpectNoError(err, "removing provisioned StorageClass") + framework.Logf("Deleted StorageClass %s", sc.Name) + }() + + pvc, pv, err := createEFSPVCPV(f.ClientSet, f.Namespace.Name, "root-dir-pvc-create", "/", map[string]string{}) + defer func() { + _ = f.ClientSet.CoreV1().PersistentVolumeClaims(f.Namespace.Name).Delete(context.TODO(), pvc.Name, metav1.DeleteOptions{}) + _ = f.ClientSet.CoreV1().PersistentVolumes().Delete(context.TODO(), pv.Name, metav1.DeleteOptions{}) + }() + framework.ExpectNoError(err, "creating root mounted pv, pvc to check") + + podSpec := e2epod.MakePod(f.Namespace.Name, nil, []*v1.PersistentVolumeClaim{pvc}, false, "") + podSpec.Spec.RestartPolicy = v1.RestartPolicyNever + pod, err := f.ClientSet.CoreV1().Pods(f.Namespace.Name).Create(context.TODO(), podSpec, metav1.CreateOptions{}) + framework.ExpectNoError(err, "creating pod") + err = e2epod.WaitForPodRunningInNamespace(f.ClientSet, pod) + framework.ExpectNoError(err, "pod started running successfully") + + provisionedPath := fmt.Sprintf("/mnt/volume1/%s/%s", basePath, dynamicPvc.Spec.VolumeName) + + perms, _, err := e2evolume.PodExec(f, pod, "stat -c \"%a\" "+provisionedPath) + framework.ExpectNoError(err, "ran stat command in /mnt/volume1 to get folder permissions") + framework.Logf("Perms Output: %s", perms) + framework.ExpectEqual(perms, fmt.Sprintf("%d", 777), "Checking File Permissions of mounted folder") + + _ = f.ClientSet.CoreV1().Pods(f.Namespace.Name).Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}) + err = e2epod.WaitForPodToDisappear(f.ClientSet, f.Namespace.Name, pod.Name, labels.Everything(), time.Second*5, time.Second*60) + framework.ExpectNoError(err, "Cleaning up no longer required pod") + }) + + ginkgo.It("should delete a directory provisioned in directory provisioning mode", func() { + basePath := "dynamic_provisioning_delete" + pvc, sc := createProvisionedDirectory(f, basePath, "directory-pvc-2") + defer func() { + err := f.ClientSet.StorageV1().StorageClasses().Delete(context.TODO(), sc.Name, metav1.DeleteOptions{}) + framework.ExpectNoError(err, "removing provisioned StorageClass") + framework.Logf("Deleted StorageClass %s", sc.Name) + }() + volumeName := pvc.Spec.VolumeName + + err := f.ClientSet.CoreV1().PersistentVolumeClaims(f.Namespace.Name).Delete(context.TODO(), pvc.Name, + metav1.DeleteOptions{}) + framework.ExpectNoError(err, "deleting pvc") + + pvc, pv, err := createEFSPVCPV(f.ClientSet, f.Namespace.Name, "root-dir-pvc-delete", "/", map[string]string{}) + defer func() { + _ = f.ClientSet.CoreV1().PersistentVolumeClaims(f.Namespace.Name).Delete(context.TODO(), pvc.Name, metav1.DeleteOptions{}) + _ = f.ClientSet.CoreV1().PersistentVolumes().Delete(context.TODO(), pv.Name, metav1.DeleteOptions{}) + }() + framework.ExpectNoError(err, "creating root mounted pv, pvc to check") + + podSpec := e2epod.MakePod(f.Namespace.Name, nil, []*v1.PersistentVolumeClaim{pvc}, false, "") + podSpec.Spec.RestartPolicy = v1.RestartPolicyNever + pod, err := f.ClientSet.CoreV1().Pods(f.Namespace.Name).Create(context.TODO(), podSpec, metav1.CreateOptions{}) + framework.ExpectNoError(err, "creating pod") + err = e2epod.WaitForPodRunningInNamespace(f.ClientSet, pod) + framework.ExpectNoError(err, "pod started running successfully") + + e2evolume.VerifyExecInPodFail(f, pod, "test -d "+"/mnt/volume1/"+basePath+"/"+volumeName, 1) + + _ = f.ClientSet.CoreV1().Pods(f.Namespace.Name).Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}) + err = e2epod.WaitForPodToDisappear(f.ClientSet, f.Namespace.Name, pod.Name, labels.Everything(), time.Second*5, time.Second*60) + framework.ExpectNoError(err, "Cleaning up no longer required pod") + + }) }) }) @@ -377,7 +470,7 @@ func createEFSPVCPV(c clientset.Interface, namespace, name, path string, volumeA } func makeEFSPVCPV(namespace, name, path string, volumeAttributes map[string]string) (*v1.PersistentVolumeClaim, *v1.PersistentVolume) { - pvc := makeEFSPVC(namespace, name) + pvc := makeEFSPVC(namespace, name, "") pv := makeEFSPV(name, path, volumeAttributes) pvc.Spec.VolumeName = pv.Name pv.Spec.ClaimRef = &v1.ObjectReference{ @@ -387,8 +480,7 @@ func makeEFSPVCPV(namespace, name, path string, volumeAttributes map[string]stri return pvc, pv } -func makeEFSPVC(namespace, name string) *v1.PersistentVolumeClaim { - storageClassName := "" +func makeEFSPVC(namespace, name, storageClassName string) *v1.PersistentVolumeClaim { return &v1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Name: name,