From 32aac47977c9f3c8bda4803f8154423b9b012277 Mon Sep 17 00:00:00 2001
From: Matt Pawelczyk <125464188+mapk-amazon@users.noreply.github.com>
Date: Tue, 20 Aug 2024 19:13:44 +0200
Subject: [PATCH] Improved tests for EFS Access Points (#6401)

* Improved tests for EFS Access Points

---------

Co-authored-by: Ryan Anderson <ndry@amazon.com>
---
 .../tests/storage/storage_common.py           | 23 ++++++++++++++-----
 .../tests/storage/test_efs.py                 |  9 +++++---
 .../pcluster.config.yaml                      |  3 ++-
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/tests/integration-tests/tests/storage/storage_common.py b/tests/integration-tests/tests/storage/storage_common.py
index 8386ae77da..4c8ee66ba6 100644
--- a/tests/integration-tests/tests/storage/storage_common.py
+++ b/tests/integration-tests/tests/storage/storage_common.py
@@ -330,7 +330,7 @@ def _write_user_data(efs_id, random_file_name, access_point_id=None):
         """  # noqa: E501
 
 
-def test_efs_correctly_mounted(remote_command_executor, mount_dir, tls=False, iam=False):
+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))
     # Following EFS instruction to check https://docs.aws.amazon.com/efs/latest/ug/encryption-in-transit.html
@@ -347,12 +347,23 @@ def test_efs_correctly_mounted(remote_command_executor, mount_dir, tls=False, ia
     # Check fstab content according to https://docs.aws.amazon.com/efs/latest/ug/automount-with-efs-mount-helper.html
     logging.info("Checking efs {0} is correctly configured in fstab".format(mount_dir))
     result = remote_command_executor.run_remote_command("cat /etc/fstab")
-    if tls and iam:  # Add a another check when tls and iam are enabled together
-        assert_that(result.stdout).matches(rf".* {mount_dir} efs _netdev,noresvport,tls,iam 0 0")
-    elif tls:
-        assert_that(result.stdout).matches(rf".* {mount_dir} efs _netdev,noresvport,tls 0 0")
+    if access_point_id:
+        # tls is always enabled with access points
+        if iam:  # Add a another check when tls and iam are enabled together
+            assert_that(result.stdout).matches(
+                rf".* {mount_dir} efs _netdev,noresvport,tls,iam,accesspoint={access_point_id} 0 0"
+            )
+        else:
+            assert_that(result.stdout).matches(
+                rf".* {mount_dir} efs _netdev,noresvport,tls,accesspoint={access_point_id} 0 0"
+            )
     else:
-        assert_that(result.stdout).matches(rf".* {mount_dir} efs _netdev,noresvport 0 0")
+        if tls and iam:  # Add a another check when tls and iam are enabled together
+            assert_that(result.stdout).matches(rf".* {mount_dir} efs _netdev,noresvport,tls,iam 0 0")
+        elif tls:
+            assert_that(result.stdout).matches(rf".* {mount_dir} efs _netdev,noresvport,tls 0 0")
+        else:
+            assert_that(result.stdout).matches(rf".* {mount_dir} efs _netdev,noresvport 0 0")
 
 
 def check_dra(
diff --git a/tests/integration-tests/tests/storage/test_efs.py b/tests/integration-tests/tests/storage/test_efs.py
index 5cf594c19d..9d09597cc6 100644
--- a/tests/integration-tests/tests/storage/test_efs.py
+++ b/tests/integration-tests/tests/storage/test_efs.py
@@ -226,6 +226,8 @@ def test_efs_access_point(
     # 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])
+    tls = True
+    iam = False
     access_point_id = efs_access_point_stack_factory(efs_fs_id=efs_filesystem_id)[0]
     if scheduler != "awsbatch":
         account_id = (
@@ -250,8 +252,8 @@ def test_efs_access_point(
                     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}"
+                            "elasticfilesystem:AccessPointArn": f"arn:{get_arn_partition(region)}:"
+                            f"elasticfilesystem:{region}:{account_id}:access-point/{access_point_id}"
                         }
                     },
                 },
@@ -279,8 +281,9 @@ def test_efs_access_point(
     remote_command_executor = RemoteCommandExecutor(cluster)
 
     mount_dir = "/" + mount_dir
+    test_efs_correctly_mounted(remote_command_executor, mount_dir, tls, iam, access_point_id)
+
     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)
 
 
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
index 712a0f0cfd..6b78c31efb 100644
--- 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
@@ -48,4 +48,5 @@ SharedStorage:
     StorageType: Efs
     EfsSettings:
       FileSystemId: {{ efs_filesystem_id }}
-      AccessPointId: {{ access_point_id }}
\ No newline at end of file
+      AccessPointId: {{ access_point_id }}
+      EncryptionInTransit: true
\ No newline at end of file