From 2543032233425f7b3bb28e55ca9a18688819885f Mon Sep 17 00:00:00 2001 From: Matt Pawelczyk <125464188+mapk-amazon@users.noreply.github.com> Date: Wed, 14 Aug 2024 21:27:49 +0200 Subject: [PATCH] Thheinen/efs accesspoints (#6381) * Add EFS access point support (aws-parallelcluster#2337) * Rename new setting, Add changelog, Fix style, fix some tests * Added tests for EFS Access Points * Improved unit test for access point * Access point requires TLS https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#mount-with-access-point * Updated snapshot to included AccessPointId * Ignored Flake8 too-long-line --------- Signed-off-by: Thomas Heinen Co-authored-by: Thomas Heinen Co-authored-by: Ryan Anderson --- CHANGELOG.md | 1 + cli/src/pcluster/aws/efs.py | 11 +++ cli/src/pcluster/config/cluster_config.py | 10 ++- cli/src/pcluster/schemas/cluster_schema.py | 10 ++- cli/src/pcluster/templates/cluster_stack.py | 5 ++ .../pcluster/templates/login_nodes_stack.py | 4 + cli/src/pcluster/templates/queues_stack.py | 4 + cli/src/pcluster/validators/efs_validators.py | 25 ++++++ .../pcluster3_config_converter.py | 1 + .../slurm.full_config.snapshot.yaml | 21 +++++ .../head_node_default.dna.json | 1 + .../test_login_nodes_dna_json/dna-1.json | 1 + .../test_login_nodes_dna_json/dna-2.json | 1 + .../test_compute_nodes_dna_json/dna-1.json | 1 + .../test_compute_nodes_dna_json/dna-2.json | 1 + .../validators/test_efs_validators.py | 73 +++++++++++++++- tests/integration-tests/configs/develop.yaml | 6 ++ tests/integration-tests/conftest.py | 28 +++++++ .../tests/storage/storage_common.py | 16 +++- .../tests/storage/test_efs.py | 83 +++++++++++++++++++ .../pcluster.config.yaml | 51 ++++++++++++ 21 files changed, 345 insertions(+), 9 deletions(-) create mode 100644 tests/integration-tests/tests/storage/test_efs/test_efs_access_point/pcluster.config.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b98435437..ba56d8782d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ CHANGELOG - Add support for ap-southeast-3 region. - Add security groups to login node network load balancer. - Add `AllowedIps` configuration for login nodes. +- Add new configuration `SharedStorage/EfsSettings/AccessPointId` to specify an optional EFS access point for a mount - Allow multiple login node pools. **BUG FIXES** diff --git a/cli/src/pcluster/aws/efs.py b/cli/src/pcluster/aws/efs.py index ed55457416..9aa3ce94df 100644 --- a/cli/src/pcluster/aws/efs.py +++ b/cli/src/pcluster/aws/efs.py @@ -80,3 +80,14 @@ def describe_file_system(self, efs_fs_id): :return: the mount_target_ids """ return self._client.describe_file_systems(FileSystemId=efs_fs_id) + + @AWSExceptionHandler.handle_client_exception + @Cache.cached + def describe_access_point(self, access_point_id): + """ + Describe access point attributes for the given EFS access point id. + + :param efaccess_point_ids_ap_id: EFS access point Id + :return: the access_point details + """ + return self._client.describe_access_points(AccessPointId=access_point_id) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index b6c5f63b14..42c7cfc16f 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -149,7 +149,7 @@ PlacementGroupCapacityTypeValidator, PlacementGroupNamingValidator, ) -from pcluster.validators.efs_validators import EfsMountOptionsValidator +from pcluster.validators.efs_validators import EfsAccessPointOptionsValidator, EfsMountOptionsValidator from pcluster.validators.feature_validators import FeatureRegionValidator from pcluster.validators.fsx_validators import ( FsxAutoImportValidator, @@ -371,6 +371,7 @@ def __init__( deletion_policy: str = None, encryption_in_transit: bool = None, iam_authorization: bool = None, + access_point_id: str = None, ): super().__init__() self.mount_dir = Resource.init_param(mount_dir) @@ -387,6 +388,7 @@ def __init__( ) self.encryption_in_transit = Resource.init_param(encryption_in_transit, default=False) self.iam_authorization = Resource.init_param(iam_authorization, default=False) + self.access_point_id = Resource.init_param(access_point_id) def _register_validators(self, context: ValidatorContext = None): # noqa: D102 #pylint: disable=unused-argument self._register_validator(SharedStorageNameValidator, name=self.name) @@ -400,6 +402,12 @@ def _register_validators(self, context: ValidatorContext = None): # noqa: D102 iam_authorization=self.iam_authorization, name=self.name, ) + self._register_validator( + EfsAccessPointOptionsValidator, + access_point_id=self.access_point_id, + file_system_id=self.file_system_id, + encryption_in_transit=self.encryption_in_transit, + ) class BaseSharedFsx(Resource): diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index 7071e3a006..05727f599d 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -321,6 +321,9 @@ class EfsSettingsSchema(BaseSchema): deletion_policy = fields.Str( validate=validate.OneOf(DELETION_POLICIES), metadata={"update_policy": UpdatePolicy.SUPPORTED} ) + access_point_id = fields.Str( + validate=validate.Regexp(r"^fsap-[0-9a-z]{17}$"), metadata={"update_policy": UpdatePolicy.UNSUPPORTED} + ) encryption_in_transit = fields.Bool(metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) iam_authorization = fields.Bool(metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) @@ -331,7 +334,12 @@ def validate_file_system_id_ignored_parameters(self, data, **kwargs): messages = [] if data.get("file_system_id") is not None: for key in data: - if key is not None and key not in ["encryption_in_transit", "iam_authorization", "file_system_id"]: + if key is not None and key not in [ + "encryption_in_transit", + "iam_authorization", + "file_system_id", + "access_point_id", + ]: messages.append(EFS_MESSAGES["errors"]["ignored_param_with_efs_fs_id"].format(efs_param=key)) if messages: raise ValidationError(message=messages) diff --git a/cli/src/pcluster/templates/cluster_stack.py b/cli/src/pcluster/templates/cluster_stack.py index 30174d13e5..4627c416b2 100644 --- a/cli/src/pcluster/templates/cluster_stack.py +++ b/cli/src/pcluster/templates/cluster_stack.py @@ -1108,6 +1108,7 @@ def _add_efs_storage(self, id: str, shared_efs: SharedEfs): shared_efs.encryption_in_transit ) self.shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"].append(shared_efs.iam_authorization) + self.shared_storage_attributes[SharedStorageType.EFS]["AccessPointIds"].append(shared_efs.access_point_id) return efs_id @@ -1287,6 +1288,10 @@ def _add_head_node(self): "efs_iam_authorizations": to_comma_separated_string( self.shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"], use_lower_case=True ), + "efs_access_point_ids": to_comma_separated_string( + self.shared_storage_attributes[SharedStorageType.EFS]["AccessPointIds"], + use_lower_case=True, + ), "fsx_fs_ids": get_shared_storage_ids_by_type(self.shared_storage_infos, SharedStorageType.FSX), "fsx_mount_names": to_comma_separated_string( self.shared_storage_attributes[SharedStorageType.FSX]["MountNames"] diff --git a/cli/src/pcluster/templates/login_nodes_stack.py b/cli/src/pcluster/templates/login_nodes_stack.py index 0cf90bbd41..b06a5ae710 100644 --- a/cli/src/pcluster/templates/login_nodes_stack.py +++ b/cli/src/pcluster/templates/login_nodes_stack.py @@ -240,6 +240,10 @@ def _add_login_nodes_pool_launch_template(self): self._shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"], use_lower_case=True, ), + "efs_access_point_ids": to_comma_separated_string( + self._shared_storage_attributes[SharedStorageType.EFS]["AccessPointIds"], + use_lower_case=True, + ), "enable_intel_hpc_platform": "true" if self._config.is_intel_hpc_platform_enabled else "false", "ephemeral_dir": DEFAULT_EPHEMERAL_DIR, "fsx_fs_ids": get_shared_storage_ids_by_type(self._shared_storage_infos, SharedStorageType.FSX), diff --git a/cli/src/pcluster/templates/queues_stack.py b/cli/src/pcluster/templates/queues_stack.py index 9f2a824a6e..19cb4a357f 100644 --- a/cli/src/pcluster/templates/queues_stack.py +++ b/cli/src/pcluster/templates/queues_stack.py @@ -335,6 +335,10 @@ def _add_compute_resource_launch_template( self._shared_storage_attributes[SharedStorageType.EFS]["IamAuthorizations"], use_lower_case=True, ), + "efs_access_point_ids": to_comma_separated_string( + self._shared_storage_attributes[SharedStorageType.EFS]["AccessPointIds"], + use_lower_case=True, + ), "fsx_fs_ids": get_shared_storage_ids_by_type(self._shared_storage_infos, SharedStorageType.FSX), "fsx_mount_names": to_comma_separated_string( self._shared_storage_attributes[SharedStorageType.FSX]["MountNames"] diff --git a/cli/src/pcluster/validators/efs_validators.py b/cli/src/pcluster/validators/efs_validators.py index 35ca33c4df..4bd93f2adf 100644 --- a/cli/src/pcluster/validators/efs_validators.py +++ b/cli/src/pcluster/validators/efs_validators.py @@ -25,3 +25,28 @@ def _validate(self, encryption_in_transit: bool, iam_authorization: bool, name: f"Please either disable IAM authorization or enable encryption in-transit for file system {name}", FailureLevel.ERROR, ) + + +class EfsAccessPointOptionsValidator(Validator): + """ + EFS Mount Options validator. + + IAM Authorization requires Encryption in Transit. + """ + + def _validate(self, access_point_id: str, file_system_id: str, encryption_in_transit: bool): + + if access_point_id and not file_system_id: + 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, + ) + + if access_point_id and not encryption_in_transit: + self._add_failure( + "An access point can only be specified when encryption in transit is enabled. " + f"Please either remove the access point id {access_point_id} or enable encryption in transit.", + FailureLevel.ERROR, + ) diff --git a/cli/src/pcluster3_config_converter/pcluster3_config_converter.py b/cli/src/pcluster3_config_converter/pcluster3_config_converter.py index 5a26fcade3..d1ccb32b0c 100644 --- a/cli/src/pcluster3_config_converter/pcluster3_config_converter.py +++ b/cli/src/pcluster3_config_converter/pcluster3_config_converter.py @@ -296,6 +296,7 @@ def convert_efs_settings(self, section_name): ("efs_kms_key_id", "KmsKeyId"), ("provisioned_throughput", "ProvisionedThroughput", "getint"), ("throughput_mode", "ThroughputMode"), + ("access_point_id", "AccessPointId"), ] efs_section, efs_dict, _section_label = self.convert_storage_base( "efs", efs_label.strip(), additional_items diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml b/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml index a3e3ae6742..91a7b59934 100644 --- a/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml +++ b/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml @@ -3888,6 +3888,7 @@ SharedStorage: Name: nameebs4 StorageType: Ebs - EfsSettings: + AccessPointId: null DeletionPolicy: Delete Encrypted: false EncryptionInTransit: false @@ -3901,6 +3902,7 @@ SharedStorage: Name: efs0 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -3914,6 +3916,7 @@ SharedStorage: Name: existing-efs10 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -3927,6 +3930,7 @@ SharedStorage: Name: existing-efs11 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -3940,6 +3944,7 @@ SharedStorage: Name: existing-efs12 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -3953,6 +3958,7 @@ SharedStorage: Name: existing-efs13 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -3966,6 +3972,7 @@ SharedStorage: Name: existing-efs14 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -3979,6 +3986,7 @@ SharedStorage: Name: existing-efs15 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -3992,6 +4000,7 @@ SharedStorage: Name: existing-efs16 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4005,6 +4014,7 @@ SharedStorage: Name: existing-efs17 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4018,6 +4028,7 @@ SharedStorage: Name: existing-efs18 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4031,6 +4042,7 @@ SharedStorage: Name: existing-efs19 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4044,6 +4056,7 @@ SharedStorage: Name: existing-efs20 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4057,6 +4070,7 @@ SharedStorage: Name: existing-efs21 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4070,6 +4084,7 @@ SharedStorage: Name: existing-efs22 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4083,6 +4098,7 @@ SharedStorage: Name: existing-efs23 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4096,6 +4112,7 @@ SharedStorage: Name: existing-efs24 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4109,6 +4126,7 @@ SharedStorage: Name: existing-efs25 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4122,6 +4140,7 @@ SharedStorage: Name: existing-efs26 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4135,6 +4154,7 @@ SharedStorage: Name: existing-efs27 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false @@ -4148,6 +4168,7 @@ SharedStorage: Name: existing-efs28 StorageType: Efs - EfsSettings: + AccessPointId: null DeletionPolicy: null Encrypted: false EncryptionInTransit: false diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_head_node_dna_json/head_node_default.dna.json b/cli/tests/pcluster/templates/test_cluster_stack/test_head_node_dna_json/head_node_default.dna.json index 0862440334..30db9a7a44 100644 --- a/cli/tests/pcluster/templates/test_cluster_stack/test_head_node_dna_json/head_node_default.dna.json +++ b/cli/tests/pcluster/templates/test_cluster_stack/test_head_node_dna_json/head_node_default.dna.json @@ -20,6 +20,7 @@ "efs_fs_ids": "", "efs_iam_authorizations": "", "efs_shared_dirs": "", + "efs_access_point_ids": "", "enable_intel_hpc_platform": "false", "ephemeral_dir": "/scratch", "fsx_dns_names": "", diff --git a/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-1.json b/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-1.json index b286ed6203..170fcdef01 100644 --- a/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-1.json +++ b/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-1.json @@ -22,6 +22,7 @@ "efs_fs_ids": "", "efs_iam_authorizations": "", "efs_shared_dirs": "", + "efs_access_point_ids": "", "enable_intel_hpc_platform": "false", "ephemeral_dir": "/scratch", "fsx_dns_names": "", diff --git a/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-2.json b/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-2.json index c67887e3a4..7164689caa 100644 --- a/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-2.json +++ b/cli/tests/pcluster/templates/test_login_nodes_stack/test_login_nodes_dna_json/dna-2.json @@ -22,6 +22,7 @@ "efs_fs_ids": "", "efs_iam_authorizations": "", "efs_shared_dirs": "", + "efs_access_point_ids": "", "enable_intel_hpc_platform": "false", "ephemeral_dir": "/scratch", "fsx_dns_names": "", diff --git a/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-1.json b/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-1.json index 69a56fa080..18140d9c0c 100644 --- a/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-1.json +++ b/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-1.json @@ -20,6 +20,7 @@ "efs_fs_ids": "", "efs_iam_authorizations": "", "efs_shared_dirs": "", + "efs_access_point_ids": "", "enable_efa": "NONE", "enable_efa_gdr": "NONE", "enable_intel_hpc_platform": "false", diff --git a/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-2.json b/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-2.json index 56d0da1a2c..2f4f04c400 100644 --- a/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-2.json +++ b/cli/tests/pcluster/templates/test_queues_stack/test_compute_nodes_dna_json/dna-2.json @@ -20,6 +20,7 @@ "efs_fs_ids": "", "efs_iam_authorizations": "", "efs_shared_dirs": "", + "efs_access_point_ids": "", "enable_efa": "NONE", "enable_efa_gdr": "NONE", "enable_intel_hpc_platform": "false", diff --git a/cli/tests/pcluster/validators/test_efs_validators.py b/cli/tests/pcluster/validators/test_efs_validators.py index 70a8408179..327a03b966 100644 --- a/cli/tests/pcluster/validators/test_efs_validators.py +++ b/cli/tests/pcluster/validators/test_efs_validators.py @@ -11,7 +11,7 @@ import pytest -from pcluster.validators.efs_validators import EfsMountOptionsValidator +from pcluster.validators.efs_validators import EfsAccessPointOptionsValidator, EfsMountOptionsValidator from tests.pcluster.validators.utils import assert_failure_messages @@ -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 " - "", + "Please either disable IAM authorization or enable encryption in-transit " + "for file system ", ), ( True, @@ -47,3 +47,70 @@ def test_efs_mount_options_validator(encryption_in_transit, iam_authorization, e encryption_in_transit, iam_authorization, "" ) assert_failure_messages(actual_failures, expected_message) + + +@pytest.mark.parametrize( + "access_point_id, file_system_id, expected_message", + [ + ( + None, + None, + None, + ), + ( + "", + None, + "An access point can only be specified when using an existing EFS file system. " + "Please either remove the access point id " + "or provide the file system id for the access point", + ), + ( + "", + "", + None, + ), + ( + None, + "", + None, + ), + ], +) +def test_efs_access_point_with_filesystem_validator(access_point_id, file_system_id, expected_message): + actual_failures = EfsAccessPointOptionsValidator().execute(access_point_id, file_system_id, True) + assert_failure_messages(actual_failures, expected_message) + + +@pytest.mark.parametrize( + "access_point_id, encryption_in_transit, expected_message", + [ + ( + None, + False, + None, + ), + ( + "", + False, + "An access point can only be specified when encryption in transit is enabled. " + "Please either remove the access point id or enable encryption in transit.", + ), + ( + "", + True, + None, + ), + ( + None, + True, + None, + ), + ], +) +def test_efs_access_point_with_filesystem_encryption_validator( + access_point_id, encryption_in_transit, expected_message +): + actual_failures = EfsAccessPointOptionsValidator().execute( + access_point_id, "", encryption_in_transit + ) + assert_failure_messages(actual_failures, expected_message) diff --git a/tests/integration-tests/configs/develop.yaml b/tests/integration-tests/configs/develop.yaml index 81fdbad379..c3d1e21d40 100644 --- a/tests/integration-tests/configs/develop.yaml +++ b/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"] diff --git a/tests/integration-tests/conftest.py b/tests/integration-tests/conftest.py index 1db53d37e7..dbcea062b4 100644 --- a/tests/integration-tests/conftest.py +++ b/tests/integration-tests/conftest.py @@ -54,6 +54,7 @@ from jinja2.sandbox import SandboxedEnvironment from troposphere import Ref, Sub, Template, ec2, resourcegroups from troposphere.ec2 import PlacementGroup +from troposphere.efs import AccessPoint as EFSAccessPoint from troposphere.efs import FileSystem as EFSFileSystem from troposphere.efs import MountTarget from troposphere.fsx import ( @@ -1786,6 +1787,33 @@ def create_efs(num=1): 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): """ diff --git a/tests/integration-tests/tests/storage/storage_common.py b/tests/integration-tests/tests/storage/storage_common.py index b8b2e2c6ae..8386ae77da 100644 --- a/tests/integration-tests/tests/storage/storage_common.py +++ b/tests/integration-tests/tests/storage/storage_common.py @@ -222,7 +222,14 @@ 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 +243,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 +319,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 diff --git a/tests/integration-tests/tests/storage/test_efs.py b/tests/integration-tests/tests/storage/test_efs.py index 8d93445276..5cf594c19d 100644 --- a/tests/integration-tests/tests/storage/test_efs.py +++ b/tests/integration-tests/tests/storage/test_efs.py @@ -201,6 +201,89 @@ def test_multiple_efs( ) +@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. + # 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}:" # noqa: E501 + f"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, diff --git a/tests/integration-tests/tests/storage/test_efs/test_efs_access_point/pcluster.config.yaml b/tests/integration-tests/tests/storage/test_efs/test_efs_access_point/pcluster.config.yaml new file mode 100644 index 0000000000..712a0f0cfd --- /dev/null +++ b/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 }} \ No newline at end of file