Added tests for EFS Access Points

Author: mapk-amazon

cli/src/pcluster/validators/efs_validators.py

@@ -18,10 +18,16 @@ class EfsMountOptionsValidator(Validator):
     IAM Authorization requires Encryption in Transit.
     """
 
-    def _validate(self, encryption_in_transit: bool, iam_authorization: bool, accesspoint_id: str, name: str):
+    def _validate(self, encryption_in_transit: bool, iam_authorization: bool, access_point_id: str, name: str):
         if iam_authorization and not encryption_in_transit:
             self._add_failure(
                 "EFS IAM authorization cannot be enabled when encryption in-transit is disabled. "
                 f"Please either disable IAM authorization or enable encryption in-transit for file system {name}",
                 FailureLevel.ERROR,
             )
+        if access_point_id and not name:
+            self._add_failure(
+                "An access point can only be specified when using an existing EFS file system. "
+                f"Please either remove the access point id {access_point_id} or provide the file system id for the access point",
+                FailureLevel.ERROR,
+            )

cli/tests/pcluster/validators/test_efs_validators.py

@@ -27,8 +27,8 @@
             False,
             True,
             "EFS IAM authorization cannot be enabled when encryption in-transit is disabled. "
-            "Please either disable IAM authorization or enable encryption in-transit for file system "
-            "<name-of-the-file-system>",
+            "Please either disable IAM authorization or enable encryption in-transit "
+            "for file system <name-of-the-file-system>",
         ),
         (
             True,
@@ -42,8 +42,42 @@
         ),
     ],
 )
-def test_efs_mount_options_validator(encryption_in_transit, iam_authorization, expected_message):
+def test_efs_mount_options_validator(
+    encryption_in_transit, iam_authorization, access_point_id, file_system_id, expected_message
+):
     actual_failures = EfsMountOptionsValidator().execute(
-        encryption_in_transit, iam_authorization, "<name-of-the-file-system>"
+        encryption_in_transit, iam_authorization, None, "<name-of-the-file-system>"
     )
     assert_failure_messages(actual_failures, expected_message)
+
+
+@pytest.mark.parametrize(
+    "access_point_id, name_of_the_file_system, expected_message",
+    [
+        (
+            None,
+            None,
+            None,
+        ),
+        (
+            "<access_point_id>",
+            None,
+            "An access point can only be specified when using an existing EFS file system. "
+            "Please either remove the access point id <access_point_id> "
+            "or provide the file system id for the access point",
+        ),
+        (
+            "<access_point_id>",
+            "<name-of-the-file-system>",
+            None,
+        ),
+        (
+            None,
+            "<name-of-the-file-system>",
+            None,
+        ),
+    ],
+)
+def test_efs_access_point_validator(access_point_id, name_of_the_file_system, expected_message):
+    actual_failures = EfsMountOptionsValidator().execute(False, False, access_point_id, name_of_the_file_system)
+    assert_failure_messages(actual_failures, expected_message)

tests/integration-tests/configs/develop.yaml

@@ -635,6 +635,12 @@ test-suites:
           instances: {{ common.INSTANCES_DEFAULT_X86 }}
           oss: ["rhel8"]
           schedulers: ["slurm"]
+    test_efs.py::test_efs_access_point:
+      dimensions:
+        - regions: ["us-east-2"]
+          instances: {{ common.INSTANCES_DEFAULT_X86 }}
+          oss: ["alinux2"]
+          schedulers: ["slurm"]
     test_raid.py::test_raid_fault_tolerance_mode:
       dimensions:
         - regions: ["cn-northwest-1"]

tests/integration-tests/conftest.py

@@ -54,7 +54,7 @@
 from jinja2.sandbox import SandboxedEnvironment
 from troposphere import Ref, Sub, Template, ec2, resourcegroups
 from troposphere.ec2 import PlacementGroup
-from troposphere.efs import FileSystem as EFSFileSystem
+from troposphere.efs import FileSystem as EFSFileSystem, AccessPoint as EFSAccessPoint
 from troposphere.efs import MountTarget
 from troposphere.fsx import (
     ClientConfigurations,
@@ -1785,6 +1785,32 @@ def create_efs(num=1):
         for stack in created_stacks:
             cfn_stacks_factory.delete_stack(stack.name, region)
 
+@pytest.fixture(scope="class")
+def efs_access_point_stack_factory(cfn_stacks_factory, request, region, vpc_stack):
+    """EFS stack contains a single efs and a single access point resource."""
+    created_stacks = []
+
+    def create_access_points(efs_fs_id, num=1):
+        ap_template = Template()
+        ap_template.set_version("2010-09-09")
+        ap_template.set_description("Access Point stack created for testing existing EFS wtith Access points")
+        access_point_resource_name = "AccessPointResourceResource"
+        for i in range(num):
+            access_point = EFSAccessPoint(f"{access_point_resource_name}{i}")
+            access_point.FileSystemId = efs_fs_id
+            ap_template.add_resource(access_point)
+        stack_name = generate_stack_name("integ-tests-efs-ap", request.config.getoption("stackname_suffix"))
+        stack = CfnStack(name=stack_name, region=region, template=ap_template.to_json())
+        cfn_stacks_factory.create_stack(stack)
+        created_stacks.append(stack)
+        return [stack.cfn_resources[f"{access_point_resource_name}{i}"] for i in range(num)]
+
+    yield create_access_points
+
+    if not request.config.getoption("no_delete"):
+        for stack in created_stacks:
+            cfn_stacks_factory.delete_stack(stack.name, region)
+
 
 @pytest.fixture(scope="class")
 def efs_mount_target_stack_factory(cfn_stacks_factory, request, region, vpc_stack):

tests/integration-tests/tests/storage/storage_common.py

@@ -222,7 +222,7 @@ def test_raid_correctly_mounted(remote_command_executor, mount_dir, volume_size)
 
 
 def write_file_into_efs(
-    region, vpc_stack: CfnVpcStack, efs_ids, request, key_name, cfn_stacks_factory, efs_mount_target_stack_factory
+    region, vpc_stack: CfnVpcStack, efs_ids, request, key_name, cfn_stacks_factory, efs_mount_target_stack_factory, access_point_id=None
 ):
     """Write file stack contains an instance to write an empty file with random name into each of the efs in efs_ids."""
     write_file_template = Template()
@@ -236,7 +236,7 @@ def write_file_into_efs(
     write_file_user_data = ""
     for efs_id in efs_ids:
         random_file_name = random_alphanumeric()
-        write_file_user_data += _write_user_data(efs_id, random_file_name)
+        write_file_user_data += _write_user_data(efs_id, random_file_name, access_point_id=access_point_id)
         random_file_names.append(random_file_name)
     user_data = f"""
         #cloud-config
@@ -312,11 +312,12 @@ def write_file_into_efs(
     return random_file_names
 
 
-def _write_user_data(efs_id, random_file_name):
+def _write_user_data(efs_id, random_file_name, access_point_id=None):
     mount_dir = "/mnt/efs/fs"
+    access_point_mount_parameter = f",accesspoint={access_point_id}" if access_point_id is not None else ""
     return f"""
         - mkdir -p {mount_dir}
-        - mount -t efs -o tls,iam {efs_id}:/ {mount_dir}
+        - mount -t efs -o tls,iam{access_point_mount_parameter} {efs_id}:/ {mount_dir} 
         - touch {mount_dir}/{random_file_name}
         - umount {mount_dir}
         """  # noqa: E501

tests/integration-tests/tests/storage/test_efs.py

@@ -200,7 +200,90 @@ def test_multiple_efs(
             encryption_in_transits,
         )
 
+@pytest.mark.usefixtures("instance")
+def test_efs_access_point(
+    os,
+    region,
+    scheduler,
+    efs_stack_factory,
+    efs_mount_target_stack_factory,
+    efs_access_point_stack_factory,
+    pcluster_config_reader,
+    clusters_factory,
+    vpc_stack,
+    request,
+    key_name,
+    cfn_stacks_factory,
+    scheduler_commands_factory,
+):
+    """
+    Test when efs_fs_id is provided in the config file, the existing efs can be correctly mounted.
+
+    To verify the efs is the existing efs, the test expects a file with random ran inside the efs mounted
+    """
+    # Names of files that will be written from separate instance. The test checks the cluster nodes can access them.
+    efs_filenames = []
+    iam_authorizations = [False, False, True] if scheduler != "awsbatch" else 3 * [False]
+    encryption_in_transits = [False, True, True] if scheduler != "awsbatch" else 3 * [False]
+    # create an additional EFS with file system policy to prevent anonymous access
+    efs_filesystem_id = efs_stack_factory()[0]
+    efs_mount_target_stack_factory([efs_filesystem_id])
+    access_point_id = efs_access_point_stack_factory(efs_fs_id=efs_filesystem_id)[0]
+    if scheduler != "awsbatch":
+        account_id = (
+            boto3.client("sts", region_name=region, endpoint_url=get_sts_endpoint(region))
+            .get_caller_identity()
+            .get("Account")
+        )
+        policy = {
+            "Version": "2012-10-17",
+            "Id": "efs-policy-denying-access-for-direct-efs-access",
+            "Statement": [
+                {
+                    "Sid": "efs-block-not-access-point-in-account",
+                    "Effect": "Deny",
+                    "Principal": {"AWS": "*"},
+                    "Action": [
+                        "elasticfilesystem:ClientMount",
+                        "elasticfilesystem:ClientRootAccess",
+                        "elasticfilesystem:ClientWrite",
+                    ],
+                    "Resource": f"arn:{get_arn_partition(region)}:elasticfilesystem:{region}:{account_id}:"
+                    f"file-system/{efs_filesystem_id}",
+                    "Condition": {
+                        "StringNotLike": {"elasticfilesystem:AccessPointArn": 
+                            f"arn:{get_arn_partition(region)}:elasticfilesystem:{region}:{account_id}:access-point/{access_point_id}"
+                        }
+                    },
+                },
+                {
+                    "Sid": "efs-allow-accesspoint-in-account",
+                    "Effect": "Allow",
+                    "Principal": {"AWS": "*"},
+                    "Action": [
+                        "elasticfilesystem:ClientMount",
+                        "elasticfilesystem:ClientRootAccess",
+                        "elasticfilesystem:ClientWrite",
+                    ],
+                    "Resource": f"arn:{get_arn_partition(region)}:elasticfilesystem:{region}:{account_id}:"
+                    f"file-system/{efs_filesystem_id}"
+                }
+            ]
+        }
+        boto3.client("efs").put_file_system_policy(FileSystemId=efs_filesystem_id, Policy=json.dumps(policy))
+
+    mount_dir = "efs_mount_dir"
+    cluster_config = pcluster_config_reader(mount_dir=mount_dir, efs_filesystem_id=efs_filesystem_id, access_point_id=access_point_id)
+    cluster = clusters_factory(cluster_config)
+    remote_command_executor = RemoteCommandExecutor(cluster)
+
+    mount_dir = "/" + mount_dir
+    scheduler_commands = scheduler_commands_factory(remote_command_executor)
+    test_efs_correctly_mounted(remote_command_executor, mount_dir)
+    _test_efs_correctly_shared(remote_command_executor, mount_dir, scheduler_commands)
+
 
+    
 def _check_efs_after_nodes_reboot(
     all_mount_dirs,
     cluster,

tests/integration-tests/tests/storage/test_efs/test_efs_access_point/pcluster.config.yaml

@@ -0,0 +1,51 @@
+Image:
+  Os: {{ os }}
+HeadNode:
+  InstanceType: {{ instance }}
+  Networking:
+    SubnetId: {{ public_subnet_ids[0] }}
+  Ssh:
+    KeyName: {{ key_name }}
+  Imds:
+    Secured: {{ imds_secured }}
+Scheduling:
+  Scheduler: {{ scheduler }}
+  {% if scheduler == "awsbatch" %}AwsBatchQueues:{% else %}SlurmQueues:{% endif %}
+    - Name: queue-0
+      ComputeResources:
+        - Name: compute-resource-0
+          {% if scheduler == "awsbatch" %}
+          InstanceTypes:
+            - {{ instance }}
+          MinvCpus: 4
+          DesiredvCpus: 4
+          {% else %}
+          Instances:
+            - InstanceType: {{ instance }}
+          MinCount: 1
+          MaxCount: 1
+          {% endif %}
+      Networking:
+        SubnetIds:
+          - {{ private_subnet_ids[1] }}
+    {% if scheduler == "slurm" %}
+    - Name: queue-1
+      ComputeResources:
+        - Name: compute-resource-0
+          Instances:
+            - InstanceType: {{ instance }}
+          MinCount: 1
+          MaxCount: 1
+      Networking:
+        SubnetIds:
+          - {% if private_subnet_ids|length >= 3 %} {{ private_subnet_ids[2] }} {% else %} {{ private_subnet_ids[1] }} {% endif %}
+    {% endif %}
+# This compute subnet would be in a different AZ than head node for regions defined in AVAILABILITY_ZONE_OVERRIDES
+# See conftest for details
+SharedStorage:
+  - MountDir: {{ mount_dir }}
+    Name: efs
+    StorageType: Efs
+    EfsSettings:
+      FileSystemId: {{ efs_filesystem_id }}
+      AccessPointId: {{ access_point_id }}