From f36a98ef84529727f30d31068b2982d076ea6f25 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Mon, 7 Jul 2025 14:17:08 -0400 Subject: [PATCH 01/25] [Efs Encryption] Add HeadNode/SharedStorageSettings/Encrypted --- cli/src/pcluster/config/cluster_config.py | 13 +++++++++++++ cli/src/pcluster/schemas/cluster_schema.py | 11 +++++++++++ cli/src/pcluster/templates/cluster_stack.py | 6 +++++- cli/src/pcluster/validators/cluster_validators.py | 15 +++++++++++++++ 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index e4ba6fd4ca..78f7d70aca 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -93,6 +93,7 @@ HeadNodeImdsValidator, HeadNodeLaunchTemplateValidator, HeadNodeMemorySizeValidator, + HeadNodeSharedStorageEncryptedValidator, HostedZoneValidator, InstanceArchitectureCompatibilityValidator, IntelHpcArchitectureValidator, @@ -852,6 +853,14 @@ def __init__(self, allowed_ips: str = None, **kwargs): self.allowed_ips = Resource.init_param(allowed_ips) +class SharedStorageSettings(Resource): + """Represent the shared storage settings.""" + + def __init__(self, encrypted: bool = False): + super().__init__() + self.encrypted = encrypted + + class Dcv(Resource): """Represent the DCV configuration.""" @@ -1435,6 +1444,7 @@ def __init__( disable_simultaneous_multithreading: bool = None, local_storage: LocalStorage = None, shared_storage_type: str = None, + shared_storage_settings: SharedStorageSettings = None, dcv: Dcv = None, custom_actions: CustomActions = None, iam: Iam = None, @@ -1453,6 +1463,7 @@ def __init__( shared_storage_type, default="Ebs", ) + self.shared_storage_settings = shared_storage_settings self.dcv = dcv self.custom_actions = custom_actions self.iam = iam or Iam(implied=True) @@ -1462,6 +1473,8 @@ def __init__( def _register_validators(self, context: ValidatorContext = None): # noqa: D102 #pylint: disable=unused-argument self._register_validator(InstanceTypeValidator, instance_type=self.instance_type) + if self.shared_storage_settings: + self._register_validator(HeadNodeSharedStorageEncryptedValidator, shared_storage_type=self.shared_storage_type, encrypted=self.shared_storage_settings.encrypted) @property def architecture(self) -> str: diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index e14e345767..67ee9fdb66 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -85,6 +85,7 @@ SharedEbs, SharedEfs, SharedFsxLustre, + SharedStorageSettings, SlurmClusterConfig, SlurmComputeResource, SlurmComputeResourceNetworking, @@ -791,6 +792,15 @@ def make_resource(self, data, **kwargs): """Generate resource.""" return HeadNodeSsh(**data) +class SharedStorageSettingsSchema(BaseSchema): + """Represent the schema of SharedStorageSettings.""" + + encrypted = fields.Bool(metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) + + @post_load + def make_resource(self, data, **kwargs): + """Generate resource.""" + return SharedStorageSettings(**data) class DcvSchema(BaseSchema): """Represent the schema of DCV.""" @@ -1363,6 +1373,7 @@ class HeadNodeSchema(BaseSchema): metadata={"update_policy": UpdatePolicy.UNSUPPORTED}, validate=validate.OneOf(["Ebs", "Efs"]), ) + shared_storage_settings = fields.Nested(SharedStorageSettingsSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) dcv = fields.Nested(DcvSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) custom_actions = fields.Nested(HeadNodeCustomActionsSchema, metadata={"update_policy": UpdatePolicy.IGNORED}) iam = fields.Nested(HeadNodeIamSchema, metadata={"update_policy": UpdatePolicy.SUPPORTED}) diff --git a/cli/src/pcluster/templates/cluster_stack.py b/cli/src/pcluster/templates/cluster_stack.py index 0381628ffd..fdaddcf66e 100644 --- a/cli/src/pcluster/templates/cluster_stack.py +++ b/cli/src/pcluster/templates/cluster_stack.py @@ -266,8 +266,12 @@ def _add_resources(self): # then it will be unmounted and the shared dirs will be # mounted. We need to create the additional mount points first. if self.config.head_node.shared_storage_type.lower() == SharedStorageType.EFS.value: + try: + encrypted = self.config.head_node.shared_storage_settings.encrypted + except AttributeError: + encrypted = False internal_efs_storage_shared = SharedEfs( - mount_dir="/opt/parallelcluster/init_shared", name="internal_pcluster_shared", throughput_mode="elastic" + mount_dir="/opt/parallelcluster/init_shared", name="internal_pcluster_shared", throughput_mode="elastic", encrypted=encrypted ) self._add_shared_storage(internal_efs_storage_shared) diff --git a/cli/src/pcluster/validators/cluster_validators.py b/cli/src/pcluster/validators/cluster_validators.py index 82bbdb17a8..91b28094bf 100644 --- a/cli/src/pcluster/validators/cluster_validators.py +++ b/cli/src/pcluster/validators/cluster_validators.py @@ -1335,6 +1335,21 @@ def _validate(self, head_node_instance_type: str, total_max_compute_nodes: int): FailureLevel.ERROR, ) +class HeadNodeSharedStorageEncryptedValidator(Validator): + """ + Head Node Shared Storage Encrypted Validator. + + Verify Head Node Shared Storage Encryption can only be used with Efs Shared Storage Type. + """ + + def _validate(self, shared_storage_type: str, encrypted: bool): + if encrypted and shared_storage_type != "Efs": + self._add_failure( + f"HeadNode is using SharedStorageSetting/Encrypted = true " + f"but the SharedStorageType specified is {shared_storage_type}. " + f"SharedStorageSetting/Encrypted = true can only be used SharedStorageType specified as Efs.", + FailureLevel.ERROR, + ) class SharedEbsPerformanceBottleNeckValidator(Validator): """Warn potential performance bottleneck of using Shared EBS.""" From ac5f9fb28aeb5fed20f44e21a84e28728b31e9e5 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 8 Jul 2025 15:14:02 -0400 Subject: [PATCH 02/25] [Efs Encryption] Update SharedStorageEfsSettingsEncryptedValidator to check that SharedStorageEfsSettings can only be used when SharedStorageType is specified as Efs --- cli/src/pcluster/config/cluster_config.py | 11 +++++------ cli/src/pcluster/schemas/cluster_schema.py | 8 ++++---- .../pcluster/validators/cluster_validators.py | 16 ++++++++-------- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 78f7d70aca..a01db7aea8 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -93,7 +93,7 @@ HeadNodeImdsValidator, HeadNodeLaunchTemplateValidator, HeadNodeMemorySizeValidator, - HeadNodeSharedStorageEncryptedValidator, + SharedStorageEfsSettingsEncryptedValidator, HostedZoneValidator, InstanceArchitectureCompatibilityValidator, IntelHpcArchitectureValidator, @@ -853,7 +853,7 @@ def __init__(self, allowed_ips: str = None, **kwargs): self.allowed_ips = Resource.init_param(allowed_ips) -class SharedStorageSettings(Resource): +class SharedStorageEfsSettings(Resource): """Represent the shared storage settings.""" def __init__(self, encrypted: bool = False): @@ -1444,7 +1444,7 @@ def __init__( disable_simultaneous_multithreading: bool = None, local_storage: LocalStorage = None, shared_storage_type: str = None, - shared_storage_settings: SharedStorageSettings = None, + shared_storage_efs_settings: SharedStorageEfsSettings = None, dcv: Dcv = None, custom_actions: CustomActions = None, iam: Iam = None, @@ -1463,7 +1463,7 @@ def __init__( shared_storage_type, default="Ebs", ) - self.shared_storage_settings = shared_storage_settings + self.shared_storage_settings = shared_storage_efs_settings self.dcv = dcv self.custom_actions = custom_actions self.iam = iam or Iam(implied=True) @@ -1473,8 +1473,7 @@ def __init__( def _register_validators(self, context: ValidatorContext = None): # noqa: D102 #pylint: disable=unused-argument self._register_validator(InstanceTypeValidator, instance_type=self.instance_type) - if self.shared_storage_settings: - self._register_validator(HeadNodeSharedStorageEncryptedValidator, shared_storage_type=self.shared_storage_type, encrypted=self.shared_storage_settings.encrypted) + self._register_validator(SharedStorageEfsSettingsEncryptedValidator, shared_storage_type=self.shared_storage_type, shared_storage_efs_settings=self.shared_storage_settings) @property def architecture(self) -> str: diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index 67ee9fdb66..1723eaec29 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -85,7 +85,7 @@ SharedEbs, SharedEfs, SharedFsxLustre, - SharedStorageSettings, + SharedStorageEfsSettings, SlurmClusterConfig, SlurmComputeResource, SlurmComputeResourceNetworking, @@ -792,7 +792,7 @@ def make_resource(self, data, **kwargs): """Generate resource.""" return HeadNodeSsh(**data) -class SharedStorageSettingsSchema(BaseSchema): +class SharedStorageEfsSettingsSchema(BaseSchema): """Represent the schema of SharedStorageSettings.""" encrypted = fields.Bool(metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) @@ -800,7 +800,7 @@ class SharedStorageSettingsSchema(BaseSchema): @post_load def make_resource(self, data, **kwargs): """Generate resource.""" - return SharedStorageSettings(**data) + return SharedStorageEfsSettings(**data) class DcvSchema(BaseSchema): """Represent the schema of DCV.""" @@ -1373,7 +1373,7 @@ class HeadNodeSchema(BaseSchema): metadata={"update_policy": UpdatePolicy.UNSUPPORTED}, validate=validate.OneOf(["Ebs", "Efs"]), ) - shared_storage_settings = fields.Nested(SharedStorageSettingsSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) + shared_storage_settings = fields.Nested(SharedStorageEfsSettingsSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) dcv = fields.Nested(DcvSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) custom_actions = fields.Nested(HeadNodeCustomActionsSchema, metadata={"update_policy": UpdatePolicy.IGNORED}) iam = fields.Nested(HeadNodeIamSchema, metadata={"update_policy": UpdatePolicy.SUPPORTED}) diff --git a/cli/src/pcluster/validators/cluster_validators.py b/cli/src/pcluster/validators/cluster_validators.py index 91b28094bf..84a15c7d3d 100644 --- a/cli/src/pcluster/validators/cluster_validators.py +++ b/cli/src/pcluster/validators/cluster_validators.py @@ -1335,19 +1335,19 @@ def _validate(self, head_node_instance_type: str, total_max_compute_nodes: int): FailureLevel.ERROR, ) -class HeadNodeSharedStorageEncryptedValidator(Validator): +class SharedStorageEfsSettingsEncryptedValidator(Validator): """ - Head Node Shared Storage Encrypted Validator. + HeadNode SharedStorageEfsSettings Validator. - Verify Head Node Shared Storage Encryption can only be used with Efs Shared Storage Type. + Verify HeadNode SharedStorageEfsSettings can only be used with Efs SharedStorageType. """ - def _validate(self, shared_storage_type: str, encrypted: bool): - if encrypted and shared_storage_type != "Efs": + def _validate(self, shared_storage_type: str, shared_storage_efs_settings): + if shared_storage_efs_settings and shared_storage_type != "Efs": self._add_failure( - f"HeadNode is using SharedStorageSetting/Encrypted = true " - f"but the SharedStorageType specified is {shared_storage_type}. " - f"SharedStorageSetting/Encrypted = true can only be used SharedStorageType specified as Efs.", + f"SharedStorageEfsSettings is specified " + f"but the SharedStorageType is set to {shared_storage_type}. " + f"SharedStorageEfsSettings can only be used when SharedStorageType is specified as Efs.", FailureLevel.ERROR, ) From df0da09066a4709e753e75e6933122fda29e26dc Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 8 Jul 2025 15:44:01 -0400 Subject: [PATCH 03/25] [Efs Encryption] Add unit tests for SharedStorageEfsSettingsEncryptedValidator's behavior and registration --- cli/src/pcluster/config/cluster_config.py | 4 +-- .../pcluster/validators/cluster_validators.py | 8 +++-- .../validators/test_all_validators.py | 9 +++++ .../validators/test_cluster_validators.py | 36 +++++++++++++++++++ 4 files changed, 52 insertions(+), 5 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index a01db7aea8..7a4723843e 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -93,7 +93,7 @@ HeadNodeImdsValidator, HeadNodeLaunchTemplateValidator, HeadNodeMemorySizeValidator, - SharedStorageEfsSettingsEncryptedValidator, + SharedStorageEfsSettingsValidator, HostedZoneValidator, InstanceArchitectureCompatibilityValidator, IntelHpcArchitectureValidator, @@ -1473,7 +1473,7 @@ def __init__( def _register_validators(self, context: ValidatorContext = None): # noqa: D102 #pylint: disable=unused-argument self._register_validator(InstanceTypeValidator, instance_type=self.instance_type) - self._register_validator(SharedStorageEfsSettingsEncryptedValidator, shared_storage_type=self.shared_storage_type, shared_storage_efs_settings=self.shared_storage_settings) + self._register_validator(SharedStorageEfsSettingsValidator, shared_storage_type=self.shared_storage_type, shared_storage_efs_settings=self.shared_storage_settings) @property def architecture(self) -> str: diff --git a/cli/src/pcluster/validators/cluster_validators.py b/cli/src/pcluster/validators/cluster_validators.py index 84a15c7d3d..eff11f72ab 100644 --- a/cli/src/pcluster/validators/cluster_validators.py +++ b/cli/src/pcluster/validators/cluster_validators.py @@ -1335,7 +1335,8 @@ def _validate(self, head_node_instance_type: str, total_max_compute_nodes: int): FailureLevel.ERROR, ) -class SharedStorageEfsSettingsEncryptedValidator(Validator): + +class SharedStorageEfsSettingsValidator(Validator): """ HeadNode SharedStorageEfsSettings Validator. @@ -1345,12 +1346,13 @@ class SharedStorageEfsSettingsEncryptedValidator(Validator): def _validate(self, shared_storage_type: str, shared_storage_efs_settings): if shared_storage_efs_settings and shared_storage_type != "Efs": self._add_failure( - f"SharedStorageEfsSettings is specified " + "SharedStorageEfsSettings is specified " f"but the SharedStorageType is set to {shared_storage_type}. " - f"SharedStorageEfsSettings can only be used when SharedStorageType is specified as Efs.", + "SharedStorageEfsSettings can only be used when SharedStorageType is specified as Efs.", FailureLevel.ERROR, ) + class SharedEbsPerformanceBottleNeckValidator(Validator): """Warn potential performance bottleneck of using Shared EBS.""" diff --git a/cli/tests/pcluster/validators/test_all_validators.py b/cli/tests/pcluster/validators/test_all_validators.py index 9e5c413bab..1dd0e5ad5d 100644 --- a/cli/tests/pcluster/validators/test_all_validators.py +++ b/cli/tests/pcluster/validators/test_all_validators.py @@ -198,6 +198,9 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) number_of_storage_validator = mocker.patch( cluster_validators + ".NumberOfStorageValidator._validate", return_value=[] ) + shared_storage_efs_settings_validator = mocker.patch( + cluster_validators + ".SharedStorageEfsSettingsValidator._validate", return_value=[] + ) deletion_policy_validator = mocker.patch(cluster_validators + ".DeletionPolicyValidator._validate", return_value=[]) root_volume_encryption_consistency_validator = mocker.patch( cluster_validators + ".RootVolumeEncryptionConsistencyValidator._validate", return_value=[] @@ -392,6 +395,12 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) ], any_order=True, ) + shared_storage_efs_settings_validator.assert_has_calls( + [ + call(shared_storage_type="Ebs", shared_storage_efs_settings=None) + ], + any_order=True, + ) # capacity reservation validators capacity_reservation_validator.assert_has_calls( [ diff --git a/cli/tests/pcluster/validators/test_cluster_validators.py b/cli/tests/pcluster/validators/test_cluster_validators.py index 0b26ec0d06..4a8ec7f96f 100644 --- a/cli/tests/pcluster/validators/test_cluster_validators.py +++ b/cli/tests/pcluster/validators/test_cluster_validators.py @@ -71,6 +71,7 @@ SchedulerDisableSudoAccessForDefaultUserValidator, SchedulerOsValidator, SharedFileCacheNotHomeValidator, + SharedStorageEfsSettingsValidator, SharedStorageMountDirValidator, SharedStorageNameValidator, UnmanagedFsxMultiAzValidator, @@ -2075,6 +2076,41 @@ def test_mixed_security_group_overwrite_validator(head_node_security_groups, que assert_failure_messages(actual_failures, expected_message) +@pytest.mark.parametrize( + "shared_storage_type, shared_storage_efs_settings, expected_message", + [ + ( + "Efs", + { + "encrypted": True + }, + None, + ), + ( + "Efs", + None, + None, + ), + ( + "Ebs", + { + "encrypted": True + }, + "SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " + "SharedStorageEfsSettings can only be used when SharedStorageType is specified as Efs.", + ), + ( + "Ebs", + None, + None, + ), + ], +) +def test_shared_storage_efs_settings_validator(shared_storage_type, shared_storage_efs_settings, expected_message): + actual_failures = SharedStorageEfsSettingsValidator().execute(shared_storage_type, shared_storage_efs_settings) + assert_failure_messages(actual_failures, expected_message) + + @pytest.mark.parametrize( "root_volume_size, ami_size, expected_message", [ From a942241614ab46a223ebf8d83a01a86b284c2364 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 8 Jul 2025 16:54:44 -0400 Subject: [PATCH 04/25] [Efs Encryption] Fix parameter name in cluster_schema.py --- cli/src/pcluster/schemas/cluster_schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index 1723eaec29..48bb68f5ae 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -1373,7 +1373,7 @@ class HeadNodeSchema(BaseSchema): metadata={"update_policy": UpdatePolicy.UNSUPPORTED}, validate=validate.OneOf(["Ebs", "Efs"]), ) - shared_storage_settings = fields.Nested(SharedStorageEfsSettingsSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) + shared_storage_efs_settings = fields.Nested(SharedStorageEfsSettingsSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) dcv = fields.Nested(DcvSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) custom_actions = fields.Nested(HeadNodeCustomActionsSchema, metadata={"update_policy": UpdatePolicy.IGNORED}) iam = fields.Nested(HeadNodeIamSchema, metadata={"update_policy": UpdatePolicy.SUPPORTED}) From 1931dc88c3c134d4380215ba870a6152f0cb996c Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Mon, 14 Jul 2025 14:29:28 -0400 Subject: [PATCH 05/25] [Efs Encryption] Fix parameter naming --- cli/src/pcluster/config/cluster_config.py | 4 ++-- cli/src/pcluster/templates/cluster_stack.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 7a4723843e..81341f5411 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -1463,7 +1463,7 @@ def __init__( shared_storage_type, default="Ebs", ) - self.shared_storage_settings = shared_storage_efs_settings + self.shared_storage_efs_settings = shared_storage_efs_settings self.dcv = dcv self.custom_actions = custom_actions self.iam = iam or Iam(implied=True) @@ -1473,7 +1473,7 @@ def __init__( def _register_validators(self, context: ValidatorContext = None): # noqa: D102 #pylint: disable=unused-argument self._register_validator(InstanceTypeValidator, instance_type=self.instance_type) - self._register_validator(SharedStorageEfsSettingsValidator, shared_storage_type=self.shared_storage_type, shared_storage_efs_settings=self.shared_storage_settings) + self._register_validator(SharedStorageEfsSettingsValidator, shared_storage_type=self.shared_storage_type, shared_storage_efs_settings=self.shared_storage_efs_settings) @property def architecture(self) -> str: diff --git a/cli/src/pcluster/templates/cluster_stack.py b/cli/src/pcluster/templates/cluster_stack.py index fdaddcf66e..b264e603b6 100644 --- a/cli/src/pcluster/templates/cluster_stack.py +++ b/cli/src/pcluster/templates/cluster_stack.py @@ -267,7 +267,7 @@ def _add_resources(self): # mounted. We need to create the additional mount points first. if self.config.head_node.shared_storage_type.lower() == SharedStorageType.EFS.value: try: - encrypted = self.config.head_node.shared_storage_settings.encrypted + encrypted = self.config.head_node.shared_storage_efs_settings.encrypted except AttributeError: encrypted = False internal_efs_storage_shared = SharedEfs( From be5ee63120ae14c0b19d279d7d1a82f7041bcde5 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Mon, 14 Jul 2025 17:40:43 -0400 Subject: [PATCH 06/25] [Efs Encryption] Add more test cases in `test_shared_storage_efs_settings_validator` --- .../validators/test_cluster_validators.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cli/tests/pcluster/validators/test_cluster_validators.py b/cli/tests/pcluster/validators/test_cluster_validators.py index 4a8ec7f96f..66c89cbc01 100644 --- a/cli/tests/pcluster/validators/test_cluster_validators.py +++ b/cli/tests/pcluster/validators/test_cluster_validators.py @@ -2086,6 +2086,13 @@ def test_mixed_security_group_overwrite_validator(head_node_security_groups, que }, None, ), + ( + "Efs", + { + "encrypted": False + }, + None, + ), ( "Efs", None, @@ -2099,6 +2106,14 @@ def test_mixed_security_group_overwrite_validator(head_node_security_groups, que "SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " "SharedStorageEfsSettings can only be used when SharedStorageType is specified as Efs.", ), + ( + "Ebs", + { + "encrypted": False + }, + "SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " + "SharedStorageEfsSettings can only be used when SharedStorageType is specified as Efs.", + ), ( "Ebs", None, From 91047d8056bfb27eb43885149322747be80e283c Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 15 Jul 2025 11:13:13 -0400 Subject: [PATCH 07/25] [Efs Encryption] Update the comments of `SharedStorageEfsSettings` and `SharedStorageEfsSettingsSchema` class to make them clearer. --- cli/src/pcluster/config/cluster_config.py | 2 +- cli/src/pcluster/schemas/cluster_schema.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 5d0544cb1f..36532376d5 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -853,7 +853,7 @@ def __init__(self, allowed_ips: str = None, **kwargs): class SharedStorageEfsSettings(Resource): - """Represent the shared storage settings.""" + """Represent the settings of Efs shared storage used by HeadNode.""" def __init__(self, encrypted: bool = False): super().__init__() diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index 48bb68f5ae..8d15a66d50 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -793,7 +793,7 @@ def make_resource(self, data, **kwargs): return HeadNodeSsh(**data) class SharedStorageEfsSettingsSchema(BaseSchema): - """Represent the schema of SharedStorageSettings.""" + """Represent the schema of SharedStorageEfsSettings for the HeadNode.""" encrypted = fields.Bool(metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) From 88b3781754d4208735a3bbef445632d9b48a6dae Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 15 Jul 2025 11:29:32 -0400 Subject: [PATCH 08/25] [Efs Encryption] Move SharedStorageType Enum from cluster_config.py to common.py. --- cli/src/pcluster/config/cluster_config.py | 11 +---------- cli/src/pcluster/config/common.py | 9 +++++++++ cli/src/pcluster/templates/awsbatch_builder.py | 3 ++- cli/src/pcluster/templates/cdk_builder_utils.py | 2 +- cli/src/pcluster/templates/cluster_stack.py | 3 +-- cli/src/pcluster/templates/cw_dashboard_builder.py | 3 ++- cli/src/pcluster/templates/login_nodes_stack.py | 4 ++-- cli/src/pcluster/templates/queues_stack.py | 4 ++-- .../pcluster/templates/test_cw_dashboard_builder.py | 2 +- 9 files changed, 21 insertions(+), 20 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 36532376d5..d9704bae06 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -28,7 +28,7 @@ BaseDevSettings, BaseTag, CapacityType, - DefaultUserHomeType, + DefaultUserHomeType, SharedStorageType, ) from pcluster.config.common import Imds as TopLevelImds from pcluster.config.common import ( @@ -292,15 +292,6 @@ def __init__(self, root_volume: RootVolume = None, ephemeral_volume: EphemeralVo self.ephemeral_volume = ephemeral_volume -class SharedStorageType(Enum): - """Define storage types to be used as shared storage.""" - - EBS = "ebs" - RAID = "raid" - EFS = "efs" - FSX = "fsx" - - class SharedEbs(Ebs): """Represent a shared EBS, inherits from both _SharedStorage and Ebs classes.""" diff --git a/cli/src/pcluster/config/common.py b/cli/src/pcluster/config/common.py index cf7f54323c..06e20c303f 100644 --- a/cli/src/pcluster/config/common.py +++ b/cli/src/pcluster/config/common.py @@ -434,3 +434,12 @@ def dump_json(self): attribute_json = {"cluster": self._cluster_attributes} attribute_json.update(self._extra_attributes) return json.dumps(attribute_json, sort_keys=True) + + +class SharedStorageType(Enum): + """Define storage types to be used as shared storage.""" + + EBS = "ebs" + RAID = "raid" + EFS = "efs" + FSX = "fsx" diff --git a/cli/src/pcluster/templates/awsbatch_builder.py b/cli/src/pcluster/templates/awsbatch_builder.py index 7e5464790d..336b23d36e 100644 --- a/cli/src/pcluster/templates/awsbatch_builder.py +++ b/cli/src/pcluster/templates/awsbatch_builder.py @@ -22,7 +22,8 @@ from aws_cdk.aws_ec2 import CfnSecurityGroup from aws_cdk.core import CfnOutput, CfnResource, Construct, Fn, Stack -from pcluster.config.cluster_config import AwsBatchClusterConfig, CapacityType, SharedStorageType +from pcluster.config.cluster_config import AwsBatchClusterConfig, CapacityType +from pcluster.config.common import SharedStorageType from pcluster.constants import AWSBATCH_CLI_REQUIREMENTS, CW_LOG_GROUP_NAME_PREFIX, IAM_ROLE_PATH from pcluster.models.s3_bucket import S3Bucket from pcluster.templates.cdk_builder_utils import ( diff --git a/cli/src/pcluster/templates/cdk_builder_utils.py b/cli/src/pcluster/templates/cdk_builder_utils.py index 6ff4200146..6c94e5ccea 100644 --- a/cli/src/pcluster/templates/cdk_builder_utils.py +++ b/cli/src/pcluster/templates/cdk_builder_utils.py @@ -29,11 +29,11 @@ BaseQueue, HeadNode, LoginNodesPool, - SharedStorageType, SlurmClusterConfig, SlurmComputeResource, SlurmQueue, ) +from pcluster.config.common import SharedStorageType from pcluster.constants import ( COOKBOOK_PACKAGES_VERSIONS, CW_LOGS_RETENTION_DAYS_DEFAULT, diff --git a/cli/src/pcluster/templates/cluster_stack.py b/cli/src/pcluster/templates/cluster_stack.py index d986d11826..f6c401a2fb 100644 --- a/cli/src/pcluster/templates/cluster_stack.py +++ b/cli/src/pcluster/templates/cluster_stack.py @@ -53,10 +53,9 @@ SharedEbs, SharedEfs, SharedFsxLustre, - SharedStorageType, SlurmClusterConfig, ) -from pcluster.config.common import DefaultUserHomeType +from pcluster.config.common import DefaultUserHomeType, SharedStorageType from pcluster.constants import ( ALL_PORTS_RANGE, CW_ALARM_DATAPOINTS_TO_ALARM_DEFAULT, diff --git a/cli/src/pcluster/templates/cw_dashboard_builder.py b/cli/src/pcluster/templates/cw_dashboard_builder.py index 0d39109426..d676739482 100644 --- a/cli/src/pcluster/templates/cw_dashboard_builder.py +++ b/cli/src/pcluster/templates/cw_dashboard_builder.py @@ -17,7 +17,8 @@ from aws_cdk.aws_cloudwatch import IAlarm from aws_cdk.core import Construct, Duration, Stack -from pcluster.config.cluster_config import BaseClusterConfig, ExistingFileCache, SharedFsxLustre, SharedStorageType +from pcluster.config.cluster_config import BaseClusterConfig, ExistingFileCache, SharedFsxLustre +from pcluster.config.common import SharedStorageType from pcluster.constants import Feature from pcluster.utils import is_feature_supported diff --git a/cli/src/pcluster/templates/login_nodes_stack.py b/cli/src/pcluster/templates/login_nodes_stack.py index c71f8a88b1..97e3de8d39 100644 --- a/cli/src/pcluster/templates/login_nodes_stack.py +++ b/cli/src/pcluster/templates/login_nodes_stack.py @@ -8,8 +8,8 @@ from aws_cdk.core import CfnTag, Construct, Fn, NestedStack, Stack, Tags from pcluster.aws.aws_api import AWSApi -from pcluster.config.cluster_config import LoginNodesPool, SharedStorageType, SlurmClusterConfig -from pcluster.config.common import DefaultUserHomeType +from pcluster.config.cluster_config import LoginNodesPool, SlurmClusterConfig +from pcluster.config.common import DefaultUserHomeType, SharedStorageType from pcluster.constants import ( DEFAULT_EPHEMERAL_DIR, NODE_BOOTSTRAP_TIMEOUT, diff --git a/cli/src/pcluster/templates/queues_stack.py b/cli/src/pcluster/templates/queues_stack.py index fc55146713..3c41bc72b0 100644 --- a/cli/src/pcluster/templates/queues_stack.py +++ b/cli/src/pcluster/templates/queues_stack.py @@ -7,8 +7,8 @@ from constructs import Construct from pcluster.aws.aws_api import AWSApi -from pcluster.config.cluster_config import SharedStorageType, SlurmClusterConfig, SlurmComputeResource, SlurmQueue -from pcluster.config.common import DefaultUserHomeType +from pcluster.config.cluster_config import SlurmClusterConfig, SlurmComputeResource, SlurmQueue +from pcluster.config.common import DefaultUserHomeType, SharedStorageType from pcluster.constants import ( DEFAULT_EPHEMERAL_DIR, NODE_BOOTSTRAP_TIMEOUT, diff --git a/cli/tests/pcluster/templates/test_cw_dashboard_builder.py b/cli/tests/pcluster/templates/test_cw_dashboard_builder.py index af3243d2dd..647a69a182 100644 --- a/cli/tests/pcluster/templates/test_cw_dashboard_builder.py +++ b/cli/tests/pcluster/templates/test_cw_dashboard_builder.py @@ -15,7 +15,7 @@ import yaml from assertpy import assert_that -from pcluster.config.cluster_config import SharedStorageType +from pcluster.config.common import SharedStorageType from pcluster.constants import Feature from pcluster.schemas.cluster_schema import ClusterSchema from pcluster.templates.cdk_builder import CDKTemplateBuilder From 7508cfcfa1cdb997fabb1bb358885b25c9df0143 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 15 Jul 2025 11:30:25 -0400 Subject: [PATCH 09/25] [Efs Encryption] Change hardcoded ShareStorageType to an enum in the validator and unit test --- .../pcluster/validators/cluster_validators.py | 10 ++++----- .../validators/test_cluster_validators.py | 22 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/cli/src/pcluster/validators/cluster_validators.py b/cli/src/pcluster/validators/cluster_validators.py index eff11f72ab..dbaeed2e84 100644 --- a/cli/src/pcluster/validators/cluster_validators.py +++ b/cli/src/pcluster/validators/cluster_validators.py @@ -20,7 +20,7 @@ from pcluster.aws.aws_resources import InstanceTypeInfo from pcluster.aws.common import AWSClientError from pcluster.cli.commands.dcv_util import get_supported_dcv_os -from pcluster.config.common import CapacityType +from pcluster.config.common import CapacityType, SharedStorageType from pcluster.constants import ( CIDR_ALL_IPS, DELETE_POLICY, @@ -1343,12 +1343,12 @@ class SharedStorageEfsSettingsValidator(Validator): Verify HeadNode SharedStorageEfsSettings can only be used with Efs SharedStorageType. """ - def _validate(self, shared_storage_type: str, shared_storage_efs_settings): - if shared_storage_efs_settings and shared_storage_type != "Efs": + def _validate(self, shared_storage_type: SharedStorageType, shared_storage_efs_settings): + if shared_storage_efs_settings and shared_storage_type != SharedStorageType.EFS: self._add_failure( "SharedStorageEfsSettings is specified " - f"but the SharedStorageType is set to {shared_storage_type}. " - "SharedStorageEfsSettings can only be used when SharedStorageType is specified as Efs.", + f"but the SharedStorageType is set to {shared_storage_type.value}. " + f"SharedStorageEfsSettings can only be used when SharedStorageType is specified as {SharedStorageType.EFS.value}.", FailureLevel.ERROR, ) diff --git a/cli/tests/pcluster/validators/test_cluster_validators.py b/cli/tests/pcluster/validators/test_cluster_validators.py index 66c89cbc01..f9f1c02f53 100644 --- a/cli/tests/pcluster/validators/test_cluster_validators.py +++ b/cli/tests/pcluster/validators/test_cluster_validators.py @@ -31,7 +31,7 @@ SlurmSettings, Tag, ) -from pcluster.config.common import CapacityType +from pcluster.config.common import CapacityType, SharedStorageType from pcluster.constants import PCLUSTER_NAME_MAX_LENGTH, PCLUSTER_NAME_MAX_LENGTH_SLURM_ACCOUNTING from pcluster.validators.cluster_validators import ( FSX_MESSAGES, @@ -2080,42 +2080,42 @@ def test_mixed_security_group_overwrite_validator(head_node_security_groups, que "shared_storage_type, shared_storage_efs_settings, expected_message", [ ( - "Efs", + SharedStorageType.EFS, { "encrypted": True }, None, ), ( - "Efs", + SharedStorageType.EFS, { "encrypted": False }, None, ), ( - "Efs", + SharedStorageType.EFS, None, None, ), ( - "Ebs", + SharedStorageType.EBS, { "encrypted": True }, - "SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " - "SharedStorageEfsSettings can only be used when SharedStorageType is specified as Efs.", + f"SharedStorageEfsSettings is specified but the SharedStorageType is set to {SharedStorageType.EBS.value}. " + f"SharedStorageEfsSettings can only be used when SharedStorageType is specified as {SharedStorageType.EFS.value}.", ), ( - "Ebs", + SharedStorageType.EBS, { "encrypted": False }, - "SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " - "SharedStorageEfsSettings can only be used when SharedStorageType is specified as Efs.", + f"SharedStorageEfsSettings is specified but the SharedStorageType is set to {SharedStorageType.EBS.value}. " + f"SharedStorageEfsSettings can only be used when SharedStorageType is specified as {SharedStorageType.EFS.value}.", ), ( - "Ebs", + SharedStorageType.EBS, None, None, ), From 944762ac1728a2abbc0a18310cc88bbf3907a0ba Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 15 Jul 2025 11:36:46 -0400 Subject: [PATCH 10/25] [Efs Encryption] Fix format issues --- cli/src/pcluster/config/cluster_config.py | 11 ++-- cli/src/pcluster/schemas/cluster_schema.py | 6 ++- cli/src/pcluster/templates/cluster_stack.py | 5 +- .../pcluster/validators/cluster_validators.py | 5 +- .../validators/test_all_validators.py | 4 +- .../validators/test_cluster_validators.py | 50 ++++++++----------- 6 files changed, 43 insertions(+), 38 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index d9704bae06..7acaae7f76 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -28,11 +28,12 @@ BaseDevSettings, BaseTag, CapacityType, - DefaultUserHomeType, SharedStorageType, + DefaultUserHomeType, ) from pcluster.config.common import Imds as TopLevelImds from pcluster.config.common import ( Resource, + SharedStorageType, ) from pcluster.constants import ( CIDR_ALL_IPS, @@ -92,7 +93,6 @@ HeadNodeImdsValidator, HeadNodeLaunchTemplateValidator, HeadNodeMemorySizeValidator, - SharedStorageEfsSettingsValidator, HostedZoneValidator, InstanceArchitectureCompatibilityValidator, IntelHpcArchitectureValidator, @@ -115,6 +115,7 @@ SchedulerValidator, SharedEbsPerformanceBottleNeckValidator, SharedFileCacheNotHomeValidator, + SharedStorageEfsSettingsValidator, SharedStorageMountDirValidator, SharedStorageNameValidator, UnmanagedFsxMultiAzValidator, @@ -1463,7 +1464,11 @@ def __init__( def _register_validators(self, context: ValidatorContext = None): # noqa: D102 #pylint: disable=unused-argument self._register_validator(InstanceTypeValidator, instance_type=self.instance_type) - self._register_validator(SharedStorageEfsSettingsValidator, shared_storage_type=self.shared_storage_type, shared_storage_efs_settings=self.shared_storage_efs_settings) + self._register_validator( + SharedStorageEfsSettingsValidator, + shared_storage_type=self.shared_storage_type, + shared_storage_efs_settings=self.shared_storage_efs_settings, + ) @property def architecture(self) -> str: diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index 8d15a66d50..08e30f4f7b 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -792,6 +792,7 @@ def make_resource(self, data, **kwargs): """Generate resource.""" return HeadNodeSsh(**data) + class SharedStorageEfsSettingsSchema(BaseSchema): """Represent the schema of SharedStorageEfsSettings for the HeadNode.""" @@ -802,6 +803,7 @@ def make_resource(self, data, **kwargs): """Generate resource.""" return SharedStorageEfsSettings(**data) + class DcvSchema(BaseSchema): """Represent the schema of DCV.""" @@ -1373,7 +1375,9 @@ class HeadNodeSchema(BaseSchema): metadata={"update_policy": UpdatePolicy.UNSUPPORTED}, validate=validate.OneOf(["Ebs", "Efs"]), ) - shared_storage_efs_settings = fields.Nested(SharedStorageEfsSettingsSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) + shared_storage_efs_settings = fields.Nested( + SharedStorageEfsSettingsSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED} + ) dcv = fields.Nested(DcvSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED}) custom_actions = fields.Nested(HeadNodeCustomActionsSchema, metadata={"update_policy": UpdatePolicy.IGNORED}) iam = fields.Nested(HeadNodeIamSchema, metadata={"update_policy": UpdatePolicy.SUPPORTED}) diff --git a/cli/src/pcluster/templates/cluster_stack.py b/cli/src/pcluster/templates/cluster_stack.py index f6c401a2fb..ab21216e18 100644 --- a/cli/src/pcluster/templates/cluster_stack.py +++ b/cli/src/pcluster/templates/cluster_stack.py @@ -270,7 +270,10 @@ def _add_resources(self): except AttributeError: encrypted = False internal_efs_storage_shared = SharedEfs( - mount_dir="/opt/parallelcluster/init_shared", name="internal_pcluster_shared", throughput_mode="elastic", encrypted=encrypted + mount_dir="/opt/parallelcluster/init_shared", + name="internal_pcluster_shared", + throughput_mode="elastic", + encrypted=encrypted, ) self._add_shared_storage(internal_efs_storage_shared) diff --git a/cli/src/pcluster/validators/cluster_validators.py b/cli/src/pcluster/validators/cluster_validators.py index dbaeed2e84..c388ffb5ba 100644 --- a/cli/src/pcluster/validators/cluster_validators.py +++ b/cli/src/pcluster/validators/cluster_validators.py @@ -676,7 +676,7 @@ def _check_file_storage(self, security_groups_by_nodes, file_storages, subnet_id self._add_failure( f"The current security group settings on file storage '{file_storage_id}' does not" " satisfy mounting requirement. The file storage must be associated to a security group" - f" that allows {direction } {protocol.upper()} traffic through ports {ports}. " + f" that allows {direction} {protocol.upper()} traffic through ports {ports}. " f"Missing ports: {missing_ports}", FailureLevel.ERROR, ) @@ -1348,7 +1348,8 @@ def _validate(self, shared_storage_type: SharedStorageType, shared_storage_efs_s self._add_failure( "SharedStorageEfsSettings is specified " f"but the SharedStorageType is set to {shared_storage_type.value}. " - f"SharedStorageEfsSettings can only be used when SharedStorageType is specified as {SharedStorageType.EFS.value}.", + "SharedStorageEfsSettings can only be used when SharedStorageType " + f"is specified as {SharedStorageType.EFS.value}.", FailureLevel.ERROR, ) diff --git a/cli/tests/pcluster/validators/test_all_validators.py b/cli/tests/pcluster/validators/test_all_validators.py index 1dd0e5ad5d..c0315210e9 100644 --- a/cli/tests/pcluster/validators/test_all_validators.py +++ b/cli/tests/pcluster/validators/test_all_validators.py @@ -396,9 +396,7 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) any_order=True, ) shared_storage_efs_settings_validator.assert_has_calls( - [ - call(shared_storage_type="Ebs", shared_storage_efs_settings=None) - ], + [call(shared_storage_type="Ebs", shared_storage_efs_settings=None)], any_order=True, ) # capacity reservation validators diff --git a/cli/tests/pcluster/validators/test_cluster_validators.py b/cli/tests/pcluster/validators/test_cluster_validators.py index f9f1c02f53..ed6e1db3f6 100644 --- a/cli/tests/pcluster/validators/test_cluster_validators.py +++ b/cli/tests/pcluster/validators/test_cluster_validators.py @@ -2080,44 +2080,38 @@ def test_mixed_security_group_overwrite_validator(head_node_security_groups, que "shared_storage_type, shared_storage_efs_settings, expected_message", [ ( - SharedStorageType.EFS, - { - "encrypted": True - }, - None, + SharedStorageType.EFS, + {"encrypted": True}, + None, ), ( - SharedStorageType.EFS, - { - "encrypted": False - }, - None, + SharedStorageType.EFS, + {"encrypted": False}, + None, ), ( - SharedStorageType.EFS, - None, - None, + SharedStorageType.EFS, + None, + None, ), ( - SharedStorageType.EBS, - { - "encrypted": True - }, - f"SharedStorageEfsSettings is specified but the SharedStorageType is set to {SharedStorageType.EBS.value}. " - f"SharedStorageEfsSettings can only be used when SharedStorageType is specified as {SharedStorageType.EFS.value}.", + SharedStorageType.EBS, + {"encrypted": True}, + f"SharedStorageEfsSettings is specified but the SharedStorageType is set to {SharedStorageType.EBS.value}. " + "SharedStorageEfsSettings can only be used when SharedStorageType " + f"is specified as {SharedStorageType.EFS.value}.", ), ( - SharedStorageType.EBS, - { - "encrypted": False - }, - f"SharedStorageEfsSettings is specified but the SharedStorageType is set to {SharedStorageType.EBS.value}. " - f"SharedStorageEfsSettings can only be used when SharedStorageType is specified as {SharedStorageType.EFS.value}.", + SharedStorageType.EBS, + {"encrypted": False}, + f"SharedStorageEfsSettings is specified but the SharedStorageType is set to {SharedStorageType.EBS.value}. " + "SharedStorageEfsSettings can only be used when SharedStorageType " + f"is specified as {SharedStorageType.EFS.value}.", ), ( - SharedStorageType.EBS, - None, - None, + SharedStorageType.EBS, + None, + None, ), ], ) From 90b26915e38d779a8cb73a50d81873330a505df7 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 15 Jul 2025 12:09:45 -0400 Subject: [PATCH 11/25] [Efs Encryption] Move the addition of internal EFS shared storage away from _add_resources to decrease complexity --- cli/src/pcluster/templates/cluster_stack.py | 27 ++++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/cli/src/pcluster/templates/cluster_stack.py b/cli/src/pcluster/templates/cluster_stack.py index ab21216e18..6a31827c3d 100644 --- a/cli/src/pcluster/templates/cluster_stack.py +++ b/cli/src/pcluster/templates/cluster_stack.py @@ -263,19 +263,9 @@ def _add_resources(self): # Add the internal use shared storage to the stack # This FS will be mounted, the shared dirs will be added, # then it will be unmounted and the shared dirs will be - # mounted. We need to create the additional mount points first. + # mounted. We need to create the additional mount points first. if self.config.head_node.shared_storage_type.lower() == SharedStorageType.EFS.value: - try: - encrypted = self.config.head_node.shared_storage_efs_settings.encrypted - except AttributeError: - encrypted = False - internal_efs_storage_shared = SharedEfs( - mount_dir="/opt/parallelcluster/init_shared", - name="internal_pcluster_shared", - throughput_mode="elastic", - encrypted=encrypted, - ) - self._add_shared_storage(internal_efs_storage_shared) + self._add_internal_efs_shared_storage() # Add user configured shared storage if self.config.shared_storage: @@ -341,6 +331,19 @@ def _add_resources(self): head_node_alarms=self.head_node_alarms, ) + def _add_internal_efs_shared_storage(self): + if self.config.head_node.shared_storage_efs_settings: + encrypted = self.config.head_node.shared_storage_efs_settings.encrypted + else: + encrypted = False + internal_efs_storage_shared = SharedEfs( + mount_dir="/opt/parallelcluster/init_shared", + name="internal_pcluster_shared", + throughput_mode="elastic", + encrypted=encrypted, + ) + self._add_shared_storage(internal_efs_storage_shared) + def _cw_metric_head_node( self, namespace, metric_name, statistic="Maximum", period_seconds=CW_ALARM_PERIOD_DEFAULT, extra_dimensions=None ): From 3fc1d57837b0bc149099b5224a5a0fbe27271bc4 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 15 Jul 2025 12:38:25 -0400 Subject: [PATCH 12/25] [Efs Encryption] Add SharedStorageEfsSettings section to the test_cluster_config_limits unit test --- .../test_cluster_config_limits/slurm.full.all_resources.yaml | 2 ++ .../test_cluster_config_limits/slurm.full_config.snapshot.yaml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full.all_resources.yaml b/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full.all_resources.yaml index 2487d95d9d..de1ab9a622 100644 --- a/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full.all_resources.yaml +++ b/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full.all_resources.yaml @@ -72,6 +72,8 @@ HeadNode: DeleteOnTermination: true EphemeralVolume: MountDir: /test + SharedStorageEfsSettings: + Encrypted: false SharedStorageType: Efs # Ebs Dcv: Enabled: true 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 eddb82b155..f16c793ed4 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 @@ -70,6 +70,8 @@ HeadNode: HttpProxyAddress: https://proxy-address:port SecurityGroups: null SubnetId: subnet-12345678 + SharedStorageEfsSettings: + Encrypted: false SharedStorageType: Efs Ssh: AllowedIps: 1.2.3.4/32 From 5ba7bbd6a20f4a13135d2c3d48342789c177b05f Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 15 Jul 2025 15:56:36 -0400 Subject: [PATCH 13/25] [Efs Encryption] Fix SharedStorageEfsSettingsValidator type issue --- .../pcluster/validators/cluster_validators.py | 6 +++--- .../validators/test_cluster_validators.py | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cli/src/pcluster/validators/cluster_validators.py b/cli/src/pcluster/validators/cluster_validators.py index c388ffb5ba..298c88881f 100644 --- a/cli/src/pcluster/validators/cluster_validators.py +++ b/cli/src/pcluster/validators/cluster_validators.py @@ -1343,11 +1343,11 @@ class SharedStorageEfsSettingsValidator(Validator): Verify HeadNode SharedStorageEfsSettings can only be used with Efs SharedStorageType. """ - def _validate(self, shared_storage_type: SharedStorageType, shared_storage_efs_settings): - if shared_storage_efs_settings and shared_storage_type != SharedStorageType.EFS: + def _validate(self, shared_storage_type: str, shared_storage_efs_settings): + if shared_storage_efs_settings and shared_storage_type.lower() != SharedStorageType.EFS.value: self._add_failure( "SharedStorageEfsSettings is specified " - f"but the SharedStorageType is set to {shared_storage_type.value}. " + f"but the SharedStorageType is set to {shared_storage_type}. " "SharedStorageEfsSettings can only be used when SharedStorageType " f"is specified as {SharedStorageType.EFS.value}.", FailureLevel.ERROR, diff --git a/cli/tests/pcluster/validators/test_cluster_validators.py b/cli/tests/pcluster/validators/test_cluster_validators.py index ed6e1db3f6..5e53ce5c60 100644 --- a/cli/tests/pcluster/validators/test_cluster_validators.py +++ b/cli/tests/pcluster/validators/test_cluster_validators.py @@ -2080,36 +2080,36 @@ def test_mixed_security_group_overwrite_validator(head_node_security_groups, que "shared_storage_type, shared_storage_efs_settings, expected_message", [ ( - SharedStorageType.EFS, + "Efs", {"encrypted": True}, None, ), ( - SharedStorageType.EFS, + "Efs", {"encrypted": False}, None, ), ( - SharedStorageType.EFS, + "Efs", None, None, ), ( - SharedStorageType.EBS, + "Ebs", {"encrypted": True}, - f"SharedStorageEfsSettings is specified but the SharedStorageType is set to {SharedStorageType.EBS.value}. " + f"SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " "SharedStorageEfsSettings can only be used when SharedStorageType " f"is specified as {SharedStorageType.EFS.value}.", ), ( - SharedStorageType.EBS, + "Ebs", {"encrypted": False}, - f"SharedStorageEfsSettings is specified but the SharedStorageType is set to {SharedStorageType.EBS.value}. " + f"SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " "SharedStorageEfsSettings can only be used when SharedStorageType " f"is specified as {SharedStorageType.EFS.value}.", ), ( - SharedStorageType.EBS, + "Ebs", None, None, ), From 9b24bfc5d4beb9344e949635c85e95207ecc76e0 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 10:46:01 -0400 Subject: [PATCH 14/25] [Efs Encryption] Add id for unit tests of shared_storage_efs_settings_validator --- .../pcluster/validators/cluster_validators.py | 2 +- .../validators/test_cluster_validators.py | 22 ++++++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/cli/src/pcluster/validators/cluster_validators.py b/cli/src/pcluster/validators/cluster_validators.py index 298c88881f..2b001f2146 100644 --- a/cli/src/pcluster/validators/cluster_validators.py +++ b/cli/src/pcluster/validators/cluster_validators.py @@ -1349,7 +1349,7 @@ def _validate(self, shared_storage_type: str, shared_storage_efs_settings): "SharedStorageEfsSettings is specified " f"but the SharedStorageType is set to {shared_storage_type}. " "SharedStorageEfsSettings can only be used when SharedStorageType " - f"is specified as {SharedStorageType.EFS.value}.", + f"is specified as Efs.", FailureLevel.ERROR, ) diff --git a/cli/tests/pcluster/validators/test_cluster_validators.py b/cli/tests/pcluster/validators/test_cluster_validators.py index 5e53ce5c60..561019bc2a 100644 --- a/cli/tests/pcluster/validators/test_cluster_validators.py +++ b/cli/tests/pcluster/validators/test_cluster_validators.py @@ -2079,39 +2079,45 @@ def test_mixed_security_group_overwrite_validator(head_node_security_groups, que @pytest.mark.parametrize( "shared_storage_type, shared_storage_efs_settings, expected_message", [ - ( + pytest.param( "Efs", {"encrypted": True}, None, + id="test Efs SharedStorageType with SharedStorageEfsSettings/encrypted is True", ), - ( + pytest.param( "Efs", {"encrypted": False}, None, + id="test Efs SharedStorageType with SharedStorageEfsSettings/encrypted is False", ), - ( + pytest.param( "Efs", None, None, + id="test Efs SharedStorageType without SharedStorageEfsSettings", ), - ( + pytest.param( "Ebs", {"encrypted": True}, f"SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " "SharedStorageEfsSettings can only be used when SharedStorageType " - f"is specified as {SharedStorageType.EFS.value}.", + f"is specified as Efs.", + id="test Ebs SharedStorageType with SharedStorageEfsSettings/encrypted is True", ), - ( + pytest.param( "Ebs", {"encrypted": False}, f"SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " "SharedStorageEfsSettings can only be used when SharedStorageType " - f"is specified as {SharedStorageType.EFS.value}.", + f"is specified as Efs.", + id="test Ebs SharedStorageType with SharedStorageEfsSettings/encrypted is False", ), - ( + pytest.param( "Ebs", None, None, + id="test Ebs SharedStorageType without SharedStorageEfsSettings", ), ], ) From 5655bd6bde3ed3f05cd8982b0b92a4924c3fc653 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 10:52:59 -0400 Subject: [PATCH 15/25] [Efs Encryption] Remove setting default value of encrypted in cluster_stack.py --- cli/src/pcluster/templates/cluster_stack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/src/pcluster/templates/cluster_stack.py b/cli/src/pcluster/templates/cluster_stack.py index 6a31827c3d..40f53fb6df 100644 --- a/cli/src/pcluster/templates/cluster_stack.py +++ b/cli/src/pcluster/templates/cluster_stack.py @@ -335,7 +335,7 @@ def _add_internal_efs_shared_storage(self): if self.config.head_node.shared_storage_efs_settings: encrypted = self.config.head_node.shared_storage_efs_settings.encrypted else: - encrypted = False + encrypted = None internal_efs_storage_shared = SharedEfs( mount_dir="/opt/parallelcluster/init_shared", name="internal_pcluster_shared", From 6902c4a0307495add0846e193925b382d165ce6f Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 11:15:56 -0400 Subject: [PATCH 16/25] [Efs Encryption] Cover case where shared_storage_efs_settings is injected in test_slurm_all_validators_are_called --- .../test_slurm_all_validators_are_called/slurm_2.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cli/tests/pcluster/validators/test_all_validators/test_slurm_all_validators_are_called/slurm_2.yaml b/cli/tests/pcluster/validators/test_all_validators/test_slurm_all_validators_are_called/slurm_2.yaml index 1dd5b5524e..52e296488d 100644 --- a/cli/tests/pcluster/validators/test_all_validators/test_slurm_all_validators_are_called/slurm_2.yaml +++ b/cli/tests/pcluster/validators/test_all_validators/test_slurm_all_validators_are_called/slurm_2.yaml @@ -7,6 +7,9 @@ HeadNode: Ssh: KeyName: ec2-key-name AllowedIps: 1.2.3.4/32 + SharedStorageType: Efs + SharedStorageEfsSettings: + Encrypted: true LoginNodes: Pools: - Name: test From 598a1631739bb25cab5d1fd369fe1875ee0680a2 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 12:41:08 -0400 Subject: [PATCH 17/25] [Efs Encryption] Add unit test to verify EFS encryption in cluster stack --- .../pcluster/templates/test_cluster_stack.py | 36 ++++++++++++++++++- .../config-default.yaml | 23 ++++++++++++ .../config-encrypted.yaml | 25 +++++++++++++ .../config-unencrypted.yaml | 25 +++++++++++++ 4 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-default.yaml create mode 100644 cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-encrypted.yaml create mode 100644 cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-unencrypted.yaml diff --git a/cli/tests/pcluster/templates/test_cluster_stack.py b/cli/tests/pcluster/templates/test_cluster_stack.py index 1beb6e477f..7a0b8af4e9 100644 --- a/cli/tests/pcluster/templates/test_cluster_stack.py +++ b/cli/tests/pcluster/templates/test_cluster_stack.py @@ -204,6 +204,41 @@ def _generate_template(cluster, capsys): assert_that(err).is_empty() # Assertion failure may become an update of dependency warning deprecations. return generated_template, cluster_assets +@pytest.mark.parametrize( + "config_file_name, expected_file_system_properties", + [ + pytest.param( + "config-default.yaml", + {'Encrypted': False, 'FileSystemTags': [{'Key': 'Name', 'Value': 'internal_pcluster_shared'}]}, + id="test default Efs shared storage without SharedStorageEfsSettings", + ), + pytest.param( + "config-encrypted.yaml", + {'Encrypted': True, 'FileSystemTags': [{'Key': 'Name', 'Value': 'internal_pcluster_shared'}]}, + id="test Efs shared storage with SharedStorageEfsSettings/Encrypted is True", + ), + pytest.param( + "config-unencrypted.yaml", + {'Encrypted': False, 'FileSystemTags': [{'Key': 'Name', 'Value': 'internal_pcluster_shared'}]}, + id="test Efs shared storage with SharedStorageEfsSettings/Encrypted is False", + ), + ], +) +def test_efs_shared_storage_encryption(mocker, test_datadir, config_file_name, expected_file_system_properties): + mock_aws_api(mocker) + # mock bucket initialization parameters + mock_bucket(mocker) + mock_bucket_object_utils(mocker) + + input_yaml = load_yaml_dict(test_datadir / config_file_name) + cluster = ClusterSchema(cluster_name="clustername").load(input_yaml) + generated_template, _ = CDKTemplateBuilder().build_cluster_template( + cluster_config=cluster, bucket=dummy_cluster_bucket(), stack_name="clustername" + ) + matched_resources = get_resources( + generated_template, type="AWS::EFS::FileSystem", properties=expected_file_system_properties + ) + assert_that(matched_resources).is_length(1) @pytest.mark.parametrize( "config_file_name", @@ -1272,7 +1307,6 @@ def test_custom_munge_key_iam_policy(mocker, test_datadir, config_file_name): mock_bucket_object_utils(mocker) input_yaml = load_yaml_dict(test_datadir / config_file_name) - cluster_config = ClusterSchema(cluster_name="clustername").load(input_yaml) generated_template, _ = CDKTemplateBuilder().build_cluster_template( diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-default.yaml b/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-default.yaml new file mode 100644 index 0000000000..ee46c5a09f --- /dev/null +++ b/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-default.yaml @@ -0,0 +1,23 @@ +Image: + Os: alinux2 +HeadNode: + InstanceType: t3.micro + Ssh: + KeyName: ec2-key-name + Networking: + SubnetId: subnet-12345678 + SharedStorageType: Efs +Scheduling: + Scheduler: awsbatch + AwsBatchQueues: + - Name: queue1 + Networking: + SubnetIds: + - subnet-12345678 + ComputeResources: + - Name: compute_resource1 + InstanceTypes: + - c4.xlarge + - c5.large|optimal|c5 + - c4 + MaxvCpus: 10 \ No newline at end of file diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-encrypted.yaml b/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-encrypted.yaml new file mode 100644 index 0000000000..5ef1329329 --- /dev/null +++ b/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-encrypted.yaml @@ -0,0 +1,25 @@ +Image: + Os: alinux2 +HeadNode: + InstanceType: t3.micro + Ssh: + KeyName: ec2-key-name + Networking: + SubnetId: subnet-12345678 + SharedStorageType: Efs + SharedStorageEfsSettings: + Encrypted: true +Scheduling: + Scheduler: awsbatch + AwsBatchQueues: + - Name: queue1 + Networking: + SubnetIds: + - subnet-12345678 + ComputeResources: + - Name: compute_resource1 + InstanceTypes: + - c4.xlarge + - c5.large|optimal|c5 + - c4 + MaxvCpus: 10 diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-unencrypted.yaml b/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-unencrypted.yaml new file mode 100644 index 0000000000..be4113a7e3 --- /dev/null +++ b/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-unencrypted.yaml @@ -0,0 +1,25 @@ +Image: + Os: alinux2 +HeadNode: + InstanceType: t3.micro + Ssh: + KeyName: ec2-key-name + Networking: + SubnetId: subnet-12345678 + SharedStorageType: Efs + SharedStorageEfsSettings: + Encrypted: False +Scheduling: + Scheduler: awsbatch + AwsBatchQueues: + - Name: queue1 + Networking: + SubnetIds: + - subnet-12345678 + ComputeResources: + - Name: compute_resource1 + InstanceTypes: + - c4.xlarge + - c5.large|optimal|c5 + - c4 + MaxvCpus: 10 From 573432cf6b94d718617d54c18364b79044b4a324 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 12:44:27 -0400 Subject: [PATCH 18/25] [Efs Encryption] Fix format issues --- cli/tests/pcluster/templates/test_cluster_stack.py | 8 +++++--- cli/tests/pcluster/validators/test_cluster_validators.py | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cli/tests/pcluster/templates/test_cluster_stack.py b/cli/tests/pcluster/templates/test_cluster_stack.py index 7a0b8af4e9..7ce61a7958 100644 --- a/cli/tests/pcluster/templates/test_cluster_stack.py +++ b/cli/tests/pcluster/templates/test_cluster_stack.py @@ -204,22 +204,23 @@ def _generate_template(cluster, capsys): assert_that(err).is_empty() # Assertion failure may become an update of dependency warning deprecations. return generated_template, cluster_assets + @pytest.mark.parametrize( "config_file_name, expected_file_system_properties", [ pytest.param( "config-default.yaml", - {'Encrypted': False, 'FileSystemTags': [{'Key': 'Name', 'Value': 'internal_pcluster_shared'}]}, + {"Encrypted": False, "FileSystemTags": [{"Key": "Name", "Value": "internal_pcluster_shared"}]}, id="test default Efs shared storage without SharedStorageEfsSettings", ), pytest.param( "config-encrypted.yaml", - {'Encrypted': True, 'FileSystemTags': [{'Key': 'Name', 'Value': 'internal_pcluster_shared'}]}, + {"Encrypted": True, "FileSystemTags": [{"Key": "Name", "Value": "internal_pcluster_shared"}]}, id="test Efs shared storage with SharedStorageEfsSettings/Encrypted is True", ), pytest.param( "config-unencrypted.yaml", - {'Encrypted': False, 'FileSystemTags': [{'Key': 'Name', 'Value': 'internal_pcluster_shared'}]}, + {"Encrypted": False, "FileSystemTags": [{"Key": "Name", "Value": "internal_pcluster_shared"}]}, id="test Efs shared storage with SharedStorageEfsSettings/Encrypted is False", ), ], @@ -240,6 +241,7 @@ def test_efs_shared_storage_encryption(mocker, test_datadir, config_file_name, e ) assert_that(matched_resources).is_length(1) + @pytest.mark.parametrize( "config_file_name", [ diff --git a/cli/tests/pcluster/validators/test_cluster_validators.py b/cli/tests/pcluster/validators/test_cluster_validators.py index 561019bc2a..aa34fa2881 100644 --- a/cli/tests/pcluster/validators/test_cluster_validators.py +++ b/cli/tests/pcluster/validators/test_cluster_validators.py @@ -31,7 +31,7 @@ SlurmSettings, Tag, ) -from pcluster.config.common import CapacityType, SharedStorageType +from pcluster.config.common import CapacityType from pcluster.constants import PCLUSTER_NAME_MAX_LENGTH, PCLUSTER_NAME_MAX_LENGTH_SLURM_ACCOUNTING from pcluster.validators.cluster_validators import ( FSX_MESSAGES, @@ -2102,7 +2102,7 @@ def test_mixed_security_group_overwrite_validator(head_node_security_groups, que {"encrypted": True}, f"SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " "SharedStorageEfsSettings can only be used when SharedStorageType " - f"is specified as Efs.", + "is specified as Efs.", id="test Ebs SharedStorageType with SharedStorageEfsSettings/encrypted is True", ), pytest.param( @@ -2110,7 +2110,7 @@ def test_mixed_security_group_overwrite_validator(head_node_security_groups, que {"encrypted": False}, f"SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " "SharedStorageEfsSettings can only be used when SharedStorageType " - f"is specified as Efs.", + "is specified as Efs.", id="test Ebs SharedStorageType with SharedStorageEfsSettings/encrypted is False", ), pytest.param( From a382fd15dc1b49d89f142a04b54c9c55951b736f Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 12:46:24 -0400 Subject: [PATCH 19/25] [Efs Encryption] Rename unit test for clarity --- cli/tests/pcluster/templates/test_cluster_stack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/tests/pcluster/templates/test_cluster_stack.py b/cli/tests/pcluster/templates/test_cluster_stack.py index 7ce61a7958..4b0b659ea1 100644 --- a/cli/tests/pcluster/templates/test_cluster_stack.py +++ b/cli/tests/pcluster/templates/test_cluster_stack.py @@ -225,7 +225,7 @@ def _generate_template(cluster, capsys): ), ], ) -def test_efs_shared_storage_encryption(mocker, test_datadir, config_file_name, expected_file_system_properties): +def test_add_efs_shared_storage(mocker, test_datadir, config_file_name, expected_file_system_properties): mock_aws_api(mocker) # mock bucket initialization parameters mock_bucket(mocker) From 85aa81a476e2f2e15ec7dc430a4e5c9932eead00 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 14:54:34 -0400 Subject: [PATCH 20/25] [Efs Encryption] Update test_internal_efs to verify that the internal shared efs is correctly encrypted --- .../tests/storage/storage_common.py | 5 ++++ .../tests/storage/test_internal_efs.py | 23 +++++++++++++++---- .../test_internal_efs/pcluster.config.yaml | 2 ++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/tests/integration-tests/tests/storage/storage_common.py b/tests/integration-tests/tests/storage/storage_common.py index 4c8ee66ba6..21f472e72d 100644 --- a/tests/integration-tests/tests/storage/storage_common.py +++ b/tests/integration-tests/tests/storage/storage_common.py @@ -329,6 +329,11 @@ def _write_user_data(efs_id, random_file_name, access_point_id=None): - umount {mount_dir} """ # noqa: E501 +def test_efs_correctly_encrypted(region, efs_id, encrypted): + client = boto3.client("efs", region_name=region) + file_systems = client.describe_file_systems(FileSystemId=efs_id).get("FileSystems") + assert_that(len(file_systems)).is_equal_to(1) + assert_that(file_systems[0]["Encrypted"]).is_equal_to(encrypted) def test_efs_correctly_mounted(remote_command_executor, mount_dir, tls=False, iam=False, access_point_id=None): # The value of the two parameters should be set according to cluster configuration parameters. diff --git a/tests/integration-tests/tests/storage/test_internal_efs.py b/tests/integration-tests/tests/storage/test_internal_efs.py index 26b0491fc5..1bc68e249d 100644 --- a/tests/integration-tests/tests/storage/test_internal_efs.py +++ b/tests/integration-tests/tests/storage/test_internal_efs.py @@ -17,22 +17,37 @@ from tests.storage.storage_common import ( test_directory_correctly_shared_between_ln_and_hn, test_efs_correctly_mounted, + test_efs_correctly_encrypted, verify_directory_correctly_shared, ) - +@pytest.mark.parametrize( + "encrypted", + [True, False], +) @pytest.mark.usefixtures("os", "scheduler", "instance") def test_internal_efs( - region, scheduler, pcluster_config_reader, architecture, clusters_factory, vpc_stack, scheduler_commands_factory + region, + scheduler, + pcluster_config_reader, + architecture, + clusters_factory, + vpc_stack, + scheduler_commands_factory, + encrypted ): + """Verify that the internal shared efs is correctly encrypted""" + cluster_config = pcluster_config_reader() + cluster = clusters_factory(cluster_config) + managed_efs_filesystem_ids = [efs_id for efs_id in cluster.cfn_outputs["EFSIds"].split(",")] + test_efs_correctly_encrypted(region, managed_efs_filesystem_ids[0], encrypted=encrypted) + """Verify the internal shared storage fs is available when set to Efs""" compute_shared_dirs = ["/opt/parallelcluster/shared", "/opt/slurm", "/home"] login_shared_dirs = ["/opt/parallelcluster/shared_login_nodes", "/opt/slurm", "/home"] if architecture == "x86_64": compute_shared_dirs.append("/opt/intel") login_shared_dirs.append("/opt/intel") - cluster_config = pcluster_config_reader() - cluster = clusters_factory(cluster_config) remote_command_executor = RemoteCommandExecutor(cluster) remote_command_executor_login_node = RemoteCommandExecutor(cluster, use_login_node=True) diff --git a/tests/integration-tests/tests/storage/test_internal_efs/test_internal_efs/pcluster.config.yaml b/tests/integration-tests/tests/storage/test_internal_efs/test_internal_efs/pcluster.config.yaml index fae66eacd3..be1717fdfd 100644 --- a/tests/integration-tests/tests/storage/test_internal_efs/test_internal_efs/pcluster.config.yaml +++ b/tests/integration-tests/tests/storage/test_internal_efs/test_internal_efs/pcluster.config.yaml @@ -13,6 +13,8 @@ LoginNodes: {% endif %} HeadNode: SharedStorageType: Efs + SharedStorageEfsSettings: + Encrypted: {{ encrypted }} InstanceType: {{ instance }} Networking: SubnetId: {{ public_subnet_id }} From f569254a42200d146b601540a3ea43b643e2825e Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 16:19:41 -0400 Subject: [PATCH 21/25] [Efs Encryption] Remove the unencrypted Efs shared storage test case from integration test --- tests/integration-tests/tests/storage/test_internal_efs.py | 7 +------ .../test_internal_efs/pcluster.config.yaml | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/integration-tests/tests/storage/test_internal_efs.py b/tests/integration-tests/tests/storage/test_internal_efs.py index 1bc68e249d..f253563675 100644 --- a/tests/integration-tests/tests/storage/test_internal_efs.py +++ b/tests/integration-tests/tests/storage/test_internal_efs.py @@ -21,10 +21,6 @@ verify_directory_correctly_shared, ) -@pytest.mark.parametrize( - "encrypted", - [True, False], -) @pytest.mark.usefixtures("os", "scheduler", "instance") def test_internal_efs( region, @@ -34,13 +30,12 @@ def test_internal_efs( clusters_factory, vpc_stack, scheduler_commands_factory, - encrypted ): """Verify that the internal shared efs is correctly encrypted""" cluster_config = pcluster_config_reader() cluster = clusters_factory(cluster_config) managed_efs_filesystem_ids = [efs_id for efs_id in cluster.cfn_outputs["EFSIds"].split(",")] - test_efs_correctly_encrypted(region, managed_efs_filesystem_ids[0], encrypted=encrypted) + test_efs_correctly_encrypted(region, managed_efs_filesystem_ids[0], encrypted=True) """Verify the internal shared storage fs is available when set to Efs""" compute_shared_dirs = ["/opt/parallelcluster/shared", "/opt/slurm", "/home"] diff --git a/tests/integration-tests/tests/storage/test_internal_efs/test_internal_efs/pcluster.config.yaml b/tests/integration-tests/tests/storage/test_internal_efs/test_internal_efs/pcluster.config.yaml index be1717fdfd..a0eafbffe2 100644 --- a/tests/integration-tests/tests/storage/test_internal_efs/test_internal_efs/pcluster.config.yaml +++ b/tests/integration-tests/tests/storage/test_internal_efs/test_internal_efs/pcluster.config.yaml @@ -14,7 +14,7 @@ LoginNodes: HeadNode: SharedStorageType: Efs SharedStorageEfsSettings: - Encrypted: {{ encrypted }} + Encrypted: true InstanceType: {{ instance }} Networking: SubnetId: {{ public_subnet_id }} From 7c9785f3c0c4a8be3cf763b291ae5fcb1e456832 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 16:22:05 -0400 Subject: [PATCH 22/25] [Efs Encryption] Simplify the SharedStorageEfsSettingsValidator error message --- cli/src/pcluster/validators/cluster_validators.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cli/src/pcluster/validators/cluster_validators.py b/cli/src/pcluster/validators/cluster_validators.py index 2b001f2146..6467a4089d 100644 --- a/cli/src/pcluster/validators/cluster_validators.py +++ b/cli/src/pcluster/validators/cluster_validators.py @@ -1348,8 +1348,7 @@ def _validate(self, shared_storage_type: str, shared_storage_efs_settings): self._add_failure( "SharedStorageEfsSettings is specified " f"but the SharedStorageType is set to {shared_storage_type}. " - "SharedStorageEfsSettings can only be used when SharedStorageType " - f"is specified as Efs.", + f"SharedStorageEfsSettings can only be used when SharedStorageType is {SharedStorageType.EFS.value.capitalize()}", FailureLevel.ERROR, ) From 5d8f7d25725440f55f90930ba3cd8e615e316753 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 16:27:25 -0400 Subject: [PATCH 23/25] [Efs Encryption] Simplify the SharedStorageEfsSettingsValidator error message --- .../pcluster/validators/test_cluster_validators.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cli/tests/pcluster/validators/test_cluster_validators.py b/cli/tests/pcluster/validators/test_cluster_validators.py index aa34fa2881..aa90fe7f2a 100644 --- a/cli/tests/pcluster/validators/test_cluster_validators.py +++ b/cli/tests/pcluster/validators/test_cluster_validators.py @@ -2100,17 +2100,15 @@ def test_mixed_security_group_overwrite_validator(head_node_security_groups, que pytest.param( "Ebs", {"encrypted": True}, - f"SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " - "SharedStorageEfsSettings can only be used when SharedStorageType " - "is specified as Efs.", + "SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " + "SharedStorageEfsSettings can only be used when SharedStorageType is Efs.", id="test Ebs SharedStorageType with SharedStorageEfsSettings/encrypted is True", ), pytest.param( "Ebs", {"encrypted": False}, - f"SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " - "SharedStorageEfsSettings can only be used when SharedStorageType " - "is specified as Efs.", + "SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " + "SharedStorageEfsSettings can only be used when SharedStorageType is Efs", id="test Ebs SharedStorageType with SharedStorageEfsSettings/encrypted is False", ), pytest.param( From fa6315ea586cbc16e905d3d25eed103969ec1696 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 16:41:20 -0400 Subject: [PATCH 24/25] [Efs Encryption] Fix format issues --- cli/src/pcluster/validators/cluster_validators.py | 3 ++- .../config-default.yaml | 0 .../config-encrypted.yaml | 0 .../config-unencrypted.yaml | 0 cli/tests/pcluster/validators/test_cluster_validators.py | 2 +- tests/integration-tests/tests/storage/storage_common.py | 2 ++ tests/integration-tests/tests/storage/test_internal_efs.py | 3 ++- 7 files changed, 7 insertions(+), 3 deletions(-) rename cli/tests/pcluster/templates/test_cluster_stack/{test_efs_shared_storage_encryption => test_add_efs_shared_storage}/config-default.yaml (100%) rename cli/tests/pcluster/templates/test_cluster_stack/{test_efs_shared_storage_encryption => test_add_efs_shared_storage}/config-encrypted.yaml (100%) rename cli/tests/pcluster/templates/test_cluster_stack/{test_efs_shared_storage_encryption => test_add_efs_shared_storage}/config-unencrypted.yaml (100%) diff --git a/cli/src/pcluster/validators/cluster_validators.py b/cli/src/pcluster/validators/cluster_validators.py index 6467a4089d..4d5ec756b6 100644 --- a/cli/src/pcluster/validators/cluster_validators.py +++ b/cli/src/pcluster/validators/cluster_validators.py @@ -1348,7 +1348,8 @@ def _validate(self, shared_storage_type: str, shared_storage_efs_settings): self._add_failure( "SharedStorageEfsSettings is specified " f"but the SharedStorageType is set to {shared_storage_type}. " - f"SharedStorageEfsSettings can only be used when SharedStorageType is {SharedStorageType.EFS.value.capitalize()}", + "SharedStorageEfsSettings can only be used when " + f"SharedStorageType is {SharedStorageType.EFS.value.capitalize()}.", FailureLevel.ERROR, ) diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-default.yaml b/cli/tests/pcluster/templates/test_cluster_stack/test_add_efs_shared_storage/config-default.yaml similarity index 100% rename from cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-default.yaml rename to cli/tests/pcluster/templates/test_cluster_stack/test_add_efs_shared_storage/config-default.yaml diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-encrypted.yaml b/cli/tests/pcluster/templates/test_cluster_stack/test_add_efs_shared_storage/config-encrypted.yaml similarity index 100% rename from cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-encrypted.yaml rename to cli/tests/pcluster/templates/test_cluster_stack/test_add_efs_shared_storage/config-encrypted.yaml diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-unencrypted.yaml b/cli/tests/pcluster/templates/test_cluster_stack/test_add_efs_shared_storage/config-unencrypted.yaml similarity index 100% rename from cli/tests/pcluster/templates/test_cluster_stack/test_efs_shared_storage_encryption/config-unencrypted.yaml rename to cli/tests/pcluster/templates/test_cluster_stack/test_add_efs_shared_storage/config-unencrypted.yaml diff --git a/cli/tests/pcluster/validators/test_cluster_validators.py b/cli/tests/pcluster/validators/test_cluster_validators.py index aa90fe7f2a..8010824f97 100644 --- a/cli/tests/pcluster/validators/test_cluster_validators.py +++ b/cli/tests/pcluster/validators/test_cluster_validators.py @@ -2108,7 +2108,7 @@ def test_mixed_security_group_overwrite_validator(head_node_security_groups, que "Ebs", {"encrypted": False}, "SharedStorageEfsSettings is specified but the SharedStorageType is set to Ebs. " - "SharedStorageEfsSettings can only be used when SharedStorageType is Efs", + "SharedStorageEfsSettings can only be used when SharedStorageType is Efs.", id="test Ebs SharedStorageType with SharedStorageEfsSettings/encrypted is False", ), pytest.param( diff --git a/tests/integration-tests/tests/storage/storage_common.py b/tests/integration-tests/tests/storage/storage_common.py index 21f472e72d..94e281d93b 100644 --- a/tests/integration-tests/tests/storage/storage_common.py +++ b/tests/integration-tests/tests/storage/storage_common.py @@ -329,12 +329,14 @@ def _write_user_data(efs_id, random_file_name, access_point_id=None): - umount {mount_dir} """ # noqa: E501 + def test_efs_correctly_encrypted(region, efs_id, encrypted): client = boto3.client("efs", region_name=region) file_systems = client.describe_file_systems(FileSystemId=efs_id).get("FileSystems") assert_that(len(file_systems)).is_equal_to(1) assert_that(file_systems[0]["Encrypted"]).is_equal_to(encrypted) + def test_efs_correctly_mounted(remote_command_executor, mount_dir, tls=False, iam=False, access_point_id=None): # The value of the two parameters should be set according to cluster configuration parameters. logging.info("Checking efs {0} is correctly mounted".format(mount_dir)) diff --git a/tests/integration-tests/tests/storage/test_internal_efs.py b/tests/integration-tests/tests/storage/test_internal_efs.py index f253563675..40fea57557 100644 --- a/tests/integration-tests/tests/storage/test_internal_efs.py +++ b/tests/integration-tests/tests/storage/test_internal_efs.py @@ -16,11 +16,12 @@ from tests.storage.storage_common import ( test_directory_correctly_shared_between_ln_and_hn, - test_efs_correctly_mounted, test_efs_correctly_encrypted, + test_efs_correctly_mounted, verify_directory_correctly_shared, ) + @pytest.mark.usefixtures("os", "scheduler", "instance") def test_internal_efs( region, From 0fb133ecefbfe5a9879098e4452c9a50ddd62072 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 16 Jul 2025 16:44:19 -0400 Subject: [PATCH 25/25] [Efs Encryption] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bbcfa0cee..251c87f544 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ CHANGELOG - Support DCV on Amazon Linux 2023. - Upgrade Python runtime used by Lambda functions to python3.12 (from python3.9). - Remove `berkshelf`. All cookbooks are local and do not need `berkshelf` dependency management. +- Add the configuration parameter `HeadNode/SharedStorageEfsSettings/Encrypted` to allow user create encrypted Efs shared storage. **BUG FIXES** - Fix an issue where Security Group validation failed when a rule contained both IPv4 ranges (IpRanges) and security group references (UserIdGroupPairs).