Improved unit test for access point

Author: mapk-amazon

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 EfsMountOptionsValidator, EfsAccessPointOptionsValidator
 from pcluster.validators.feature_validators import FeatureRegionValidator
 from pcluster.validators.fsx_validators import (
     FsxAutoImportValidator,
@@ -400,10 +400,13 @@ def _register_validators(self, context: ValidatorContext = None):  # noqa: D102
             EfsMountOptionsValidator,
             encryption_in_transit=self.encryption_in_transit,
             iam_authorization=self.iam_authorization,
-            access_point_id=self.access_point_id,
             name=self.name,
         )
-
+        self._register_validator(
+            EfsAccessPointOptionsValidator,
+            access_point_id=self.access_point_id,
+            file_system_id=self.file_system_id,
+        )
 
 class BaseSharedFsx(Resource):
     """Represent the shared FSX resource."""

cli/src/pcluster/schemas/cluster_schema.py

@@ -322,7 +322,7 @@ class EfsSettingsSchema(BaseSchema):
         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.SUPPORTED}
+        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})

cli/src/pcluster/validators/efs_validators.py

@@ -18,14 +18,24 @@ class EfsMountOptionsValidator(Validator):
     IAM Authorization requires Encryption in Transit.
     """
 
-    def _validate(self, encryption_in_transit: bool, iam_authorization: bool, access_point_id: str, name: str):
+    def _validate(self, encryption_in_transit: bool, iam_authorization: bool, name: str):
         if iam_authorization and not encryption_in_transit:
             self._add_failure(
                 "EFS IAM authorization cannot be enabled when encryption in-transit is disabled. "
                 f"Please either disable IAM authorization or enable encryption in-transit for file system {name}",
                 FailureLevel.ERROR,
             )
-        if access_point_id and not name:
+
+class EfsAccessPointOptionsValidator(Validator):
+    """
+    EFS Mount Options validator.
+
+    IAM Authorization requires Encryption in Transit.
+    """
+
+    def _validate(self,  access_point_id: str, file_system_id: str):
+
+        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",

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 EfsMountOptionsValidator, EfsAccessPointOptionsValidator
 from tests.pcluster.validators.utils import assert_failure_messages
 
 
@@ -43,16 +43,16 @@
     ],
 )
 def test_efs_mount_options_validator(
-    encryption_in_transit, iam_authorization, access_point_id, file_system_id, expected_message
+    encryption_in_transit, iam_authorization, expected_message
 ):
     actual_failures = EfsMountOptionsValidator().execute(
-        encryption_in_transit, iam_authorization, None, "<name-of-the-file-system>"
+        encryption_in_transit, iam_authorization, "<name-of-the-file-system>"
     )
     assert_failure_messages(actual_failures, expected_message)
 
 
 @pytest.mark.parametrize(
-    "access_point_id, name_of_the_file_system, expected_message",
+    "access_point_id, file_system_id, expected_message",
     [
         (
             None,
@@ -68,16 +68,18 @@ def test_efs_mount_options_validator(
         ),
         (
             "<access_point_id>",
-            "<name-of-the-file-system>",
+            "<file-systemd-id>",
             None,
         ),
         (
             None,
-            "<name-of-the-file-system>",
+            "<file-systemd-id>",
             None,
         ),
     ],
 )
-def test_efs_access_point_validator(access_point_id, name_of_the_file_system, expected_message):
-    actual_failures = EfsMountOptionsValidator().execute(False, False, access_point_id, name_of_the_file_system)
+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)
     assert_failure_messages(actual_failures, expected_message)
+
+