Fix/duplicate efs entries (#694)

.pre-commit-config.yaml

@@ -19,19 +19,19 @@ repos:
       - id: fix-byte-order-marker
 
   - repo: https://github.com/asottile/pyupgrade
-    rev: v3.9.0
+    rev: v3.13.0
     hooks:
       - id: pyupgrade
         exclude: ^tests/
         args: [ "--py38-plus", "--keep-runtime-typing" ]
 
   - repo: https://github.com/psf/black
-    rev: 23.7.0
+    rev: 23.9.1
     hooks:
       - id: black
 
   - repo: https://github.com/asottile/blacken-docs
-    rev: 1.15.0
+    rev: 1.16.0
     hooks:
       - id: blacken-docs
         additional_dependencies: [ black ]
@@ -58,12 +58,12 @@ repos:
 #        files: .cicd/.*\.(json|yml|yaml)$
 
   - repo: https://github.com/hadolint/hadolint
-    rev: v2.12.0
+    rev: v2.12.1-beta
     hooks:
       - id: hadolint-docker
         args: [ "-t", "error" ]
 
   - repo: https://github.com/iamthefij/docker-pre-commit
-    rev: master
+    rev: v3.0.1
     hooks:
       - id: docker-compose-check

ecs_composex/common/settings.py

@@ -365,8 +365,6 @@ def set_efs(self) -> None:
         else:
             efs = self.compose_content["x-efs"]
         for volume in self.compose_content[ComposeVolume.main_key].values():
-            if volume.lookup or volume.use:
-                continue
             if (
                 volume.efs_definition
                 or volume.driver == "nfs"

ecs_composex/common/troposphere_tools.py

@@ -202,7 +202,7 @@ def add_resource(template, resource, replace=False) -> AWSObject:
 
 
 def add_defaults(template):
-    """Function to CFN parameters and conditions to the template whhich are used
+    """Function to CFN parameters and conditions to the template which are used
     across ECS ComposeX
 
     :param template: source template to add the params and conditions to

ecs_composex/compose/compose_volumes/__init__.py

@@ -13,7 +13,6 @@
 from compose_x_common.compose_x_common import keyisset
 
 from ecs_composex.common.logging import LOG
-from ecs_composex.efs.efs_params import FS_REGEXP
 from ecs_composex.efs.efs_params import RES_KEY as EFS_KEY
 
 from .helpers import evaluate_plugin_efs_properties
@@ -65,19 +64,12 @@ def __init__(self, name, definition):
             )
             self.type = "bind"
             self.driver = "nfs"
-        elif (
-            keyisset("external", self.definition)
-            and keyisset("name", self.definition)
-            and FS_REGEXP.match(self.definition["name"])
-        ):
-            LOG.warning(f"volumes.{self.name} - Identified a EFS to use")
-            self.efs_definition = {"Use": self.definition["name"]}
-            self.use = self.definition["name"]
         else:
             self.import_volume_from_definition()
 
     def import_volume_from_definition(self):
         if keyisset(EFS_KEY, self.definition):
+            self.efs_definition = self.definition[EFS_KEY]
             self.import_from_x_efs_settings()
         elif (
             not keyisset(EFS_KEY, self.definition)
@@ -96,9 +88,7 @@ def import_from_x_efs_settings(self):
         self.is_shared = True
         if keyisset("Lookup", self.efs_definition):
             self.lookup = self.efs_definition["Lookup"]
-        elif keyisset("Use", self.efs_definition):
-            self.use = self.efs_definition["Use"]
-        if not self.use and not self.lookup:
+        else:
             self.efs_definition = (
                 self.definition[EFS_KEY]["Properties"]
                 if keyisset("Properties", self.efs_definition)

ecs_composex/ecs_composex.py

@@ -184,7 +184,7 @@ def add_x_resources(settings: ComposeXSettings) -> None:
             Parameters={ROOT_STACK_NAME.title: Ref(AWS_STACK_NAME)},
         )
         if x_stack and x_stack.is_void:
-            settings.x_resources_void.append({module.res_key: x_stack})
+            settings.x_resources_void.append({module.mod_key: x_stack})
         elif (
             x_stack
             and hasattr(x_stack, "title")

ecs_composex/efs/efs_ecs.py

@@ -1,17 +1,29 @@
 # SPDX-License-Identifier: MPL-2.0
 # Copyright 2020-2022 John Mille <[email protected]>
 
+from __future__ import annotations
+
+from typing import TYPE_CHECKING, Union
+
+if TYPE_CHECKING:
+    from ecs_composex.common.settings import ComposeXSettings
+    from ecs_composex.ecs.ecs_family import ComposeFamily
+    from ecs_composex.efs.efs_stack import Efs
+
 from compose_x_common.compose_x_common import keyisset
 from troposphere import GetAtt, Ref
 from troposphere.ecs import AuthorizationConfig, EFSVolumeConfiguration, Volume
 from troposphere.efs import AccessPoint, CreationInfo, PosixUser, RootDirectory
 from troposphere.iam import PolicyType
 
 from ecs_composex.common.logging import LOG
-from ecs_composex.common.troposphere_tools import add_parameters
+from ecs_composex.common.troposphere_tools import add_parameters, add_resource
 from ecs_composex.ecs.ecs_params import TASK_T
 from ecs_composex.efs.efs_params import FS_ARN, FS_ID, FS_MNT_PT_SG_ID, FS_PORT
-from ecs_composex.rds_resources_settings import handle_new_tcp_resource
+from ecs_composex.rds_resources_settings import (
+    add_security_group_ingress,
+    handle_new_tcp_resource,
+)
 
 
 def get_volumes(task_definition):
@@ -86,7 +98,7 @@ def add_task_iam_access_to_access_point(family, access_points, efs):
         },
         Roles=[family.iam_manager.task_role.name],
     )
-    family.template.add_resource(policy)
+    add_resource(family.template, policy)
 
 
 def add_efs_definition_to_target_family(new_efs, target):
@@ -137,7 +149,7 @@ def override_service_volume(new_efs, fs_id, target, access_points, volumes):
                         Path=mount_pt.ContainerPath,
                     ),
                 )
-                target[0].template.add_resource(sub_service_specific_access_point)
+                add_resource(target[0].template, sub_service_specific_access_point)
                 access_points.append(sub_service_specific_access_point)
                 volumes.append(
                     Volume(
@@ -212,40 +224,56 @@ def override_efs_settings(new_efs, target, fs_id_parameter, access_points, volum
                 )
 
 
-def expand_family_with_efs_volumes(efs_root_stack_title, new_efs, settings):
+def looked_up_efs_family_hook(
+    efs: Efs, family: ComposeFamily, settings: ComposeXSettings
+) -> None:
+    sg_id = efs.add_attribute_to_another_stack(family.stack, FS_MNT_PT_SG_ID, settings)
+    add_parameters(family.template, [sg_id["ImportParameter"]])
+    add_security_group_ingress(
+        family.stack, efs.logical_name, Ref(sg_id["ImportParameter"]), 2049
+    )
+    family.stack.Parameters.update(
+        {sg_id["ImportParameter"].title: sg_id["ImportValue"]}
+    )
+
+
+def expand_family_with_efs_volumes(
+    efs_root_stack_title: str, efs: Efs, settings: ComposeXSettings
+):
     """
     Function to add the EFS Volume definition to the task definition for the service to use.
-
-    :param efs_root_stack_title: Root stack title for EFS
-    :param new_efs:
-    :param ecs_composex.common.settings.ComposeXSettings settings:
-    :return:
     """
-    fs_id_parameter = new_efs.attributes_outputs[FS_ID]["ImportParameter"]
-    fs_id_getatt = new_efs.attributes_outputs[FS_ID]["ImportValue"]
-    for target in new_efs.families_targets:
-        if target[0].service_compute.launch_type == "EXTERNAL":
+    fs_id_parameter = efs.attributes_outputs[FS_ID]["ImportParameter"]
+    fs_id_getatt = efs.attributes_outputs[FS_ID]["ImportValue"]
+    for target in efs.families_targets:
+        family: ComposeFamily = target[0]
+        if family.service_compute.launch_type == "EXTERNAL":
             LOG.warning(
-                f"x-efs - {target[0].name} - When using EXTERNAL Launch Type, networking settings cannot be set."
+                f"x-efs - {family.name} - When using EXTERNAL Launch Type, networking settings cannot be set."
             )
             return
+        if efs.lookup:
+            looked_up_efs_family_hook(efs, target[0], settings)
         access_points = []
-        target[0].stack.Parameters.update({fs_id_parameter.title: fs_id_getatt})
-        add_parameters(target[0].template, [fs_id_parameter])
-        task_definition = target[0].template.resources[TASK_T]
+        family.stack.Parameters.update({fs_id_parameter.title: fs_id_getatt})
+        add_parameters(family.template, [fs_id_parameter])
+        task_definition = family.template.resources[TASK_T]
         efs_config_kwargs = {"FilesystemId": Ref(fs_id_parameter)}
         if (
-            new_efs.parameters
-            and keyisset("EnforceIamAuth", new_efs.parameters)
+            efs.parameters
+            and keyisset("EnforceIamAuth", efs.parameters)
             or [service.user for service in target[2]]
         ):
-            add_efs_definition_to_target_family(new_efs, target)
-            efs_access_point = target[0].template.add_resource(
+            add_efs_definition_to_target_family(efs, target)
+            efs_access_point = add_resource(
+                family.template,
                 AccessPoint(
-                    f"{new_efs.logical_name}{target[0].logical_name}EfsAccessPoint",
+                    f"{efs.logical_name}{family.logical_name}EfsAccessPoint",
                     FileSystemId=Ref(fs_id_parameter),
-                )
+                ),
             )
+            if not efs_access_point:
+                continue
             access_points.append(efs_access_point)
             efs_config_kwargs.update(
                 {
@@ -257,24 +285,16 @@ def expand_family_with_efs_volumes(efs_root_stack_title, new_efs, settings):
             )
         efs_volume_definition = Volume(
             EFSVolumeConfiguration=EFSVolumeConfiguration(**efs_config_kwargs),
-            Name=new_efs.volume.volume_name,
+            Name=efs.volume.volume_name,
         )
         volumes = get_volumes(task_definition)
         volumes.append(efs_volume_definition)
-        override_efs_settings(new_efs, target, fs_id_parameter, access_points, volumes)
-        add_task_iam_access_to_access_point(target[0], access_points, new_efs)
+        override_efs_settings(efs, target, fs_id_parameter, access_points, volumes)
+        add_task_iam_access_to_access_point(family, access_points, efs)
 
 
 def efs_to_ecs(resources, services_stack, res_root_stack, settings):
-    """
-    Function to associate back the EFS FS to services.
-
-    :param resources:
-    :param services_stack:
-    :param res_root_stack:
-    :param ecs_composex.common.settings.ComposeXSettings settings:
-    :return:
-    """
+    """Function to associate back the EFS FS to services."""
     for resource_name, resource in resources.items():
         LOG.info(f"{resource.module.res_key}.{resource_name} - Linking to services")
         if not resource.mappings and resource.cfn_resource:
@@ -285,3 +305,5 @@ def efs_to_ecs(resources, services_stack, res_root_stack, settings):
                 settings=settings,
             )
             expand_family_with_efs_volumes(res_root_stack.title, resource, settings)
+        else:
+            expand_family_with_efs_volumes(None, resource, settings)

ecs_composex/efs/efs_params.py

@@ -37,3 +37,8 @@
 
 FS_MNT_PT_SG_ID_T = "FilesystemMountPointSgId"
 FS_MNT_PT_SG_ID = Parameter(FS_MNT_PT_SG_ID_T, return_value="GroupId", Type=SG_ID_TYPE)
+
+CONTROL_CLOUD_ATTR_MAPPING = {
+    FS_ID: "FileSystemId",
+    FS_ARN: "Arn",
+}

ecs_composex/efs/efs_stack.py

@@ -16,19 +16,22 @@
 
 import warnings
 
+from compose_x_common.aws.efs import EFS_ARN_RE, list_efs_mount_targets
 from troposphere import GetAtt, Ref, Select, Sub
 from troposphere.ec2 import SecurityGroup
 from troposphere.efs import FileSystem, MountTarget
 
+from ecs_composex.common.cfn_params import STACK_ID_SHORT
 from ecs_composex.common.stacks import ComposeXStack
 from ecs_composex.common.troposphere_tools import build_template
-from ecs_composex.compose.x_resources.helpers import (
-    set_lookup_resources,
-    set_new_resources,
-    set_resources,
-)
 from ecs_composex.compose.x_resources.network_x_resources import NetworkXResource
-from ecs_composex.efs.efs_params import FS_ARN, FS_ID, FS_MNT_PT_SG_ID, FS_PORT
+from ecs_composex.efs.efs_params import (
+    CONTROL_CLOUD_ATTR_MAPPING,
+    FS_ARN,
+    FS_ID,
+    FS_MNT_PT_SG_ID,
+    FS_PORT,
+)
 from ecs_composex.resources_import import import_record_properties
 from ecs_composex.vpc.vpc_params import STORAGE_SUBNETS, VPC_ID
 
@@ -48,7 +51,7 @@ def create_efs_stack(settings, new_resources):
         res.cfn_resource = FileSystem(res.logical_name, **res_cfn_props)
         res.db_sg = SecurityGroup(
             f"{res.logical_name}SecurityGroup",
-            GroupName=Sub(f"{res.logical_name}EfsSg"),
+            GroupName=Sub(f"{res.logical_name}-${{STACK_ID}}", STACK_ID=STACK_ID_SHORT),
             GroupDescription=Sub(f"SG for EFS {res.cfn_resource.title}"),
             VpcId=Ref(VPC_ID),
         )
@@ -82,6 +85,7 @@ def __init__(
         self.arn_parameter = FS_ARN
         self.security_group_param = FS_MNT_PT_SG_ID
         self.port_param = FS_PORT
+        # self.cloud_control_attributes_mapping = CONTROL_CLOUD_ATTR_MAPPING
 
     def init_outputs(self):
         """
@@ -140,6 +144,49 @@ def update_from_vpc(self, vpc_stack, settings=None):
             )
 
 
+def get_efs_details(efs: Efs, account_id, resource_id: str) -> dict:
+    client = efs.lookup_session.client("efs")
+    props: dict = {}
+    efs_r = client.describe_file_systems(FileSystemId=efs.arn)["FileSystems"][0]
+    props[FS_ARN] = efs_r["FileSystemArn"]
+    props[FS_ID] = efs_r["FileSystemId"]
+    mount_points: list = list_efs_mount_targets(
+        session=efs.lookup_session, client=client, FileSystemId=efs.arn
+    )
+    if not mount_points:
+        raise LookupError(
+            "{}.{} - No EFS MountTargets for {}".format(
+                efs.module.res_key, efs.name, efs.arn
+            )
+        )
+    groups: list = []
+    for _mnt in mount_points:
+        groups_r = client.describe_mount_target_security_groups(
+            MountTargetId=_mnt["MountTargetId"]
+        )
+        for s_group in groups_r["SecurityGroups"]:
+            if s_group not in groups:
+                groups.append(s_group)
+    if not groups:
+        raise LookupError(f"Unable to find groups for MountTargets of {efs.arn}")
+    props[FS_MNT_PT_SG_ID] = groups[0]
+    return props
+
+
+def lookup_resource(module, resource: Efs, settings: ComposeXSettings):
+    resource.lookup_resource(
+        EFS_ARN_RE,
+        get_efs_details,
+        FileSystem.resource_type,
+        "elasticfilesystem",
+    )
+    resource.generate_cfn_mappings_from_lookup_properties()
+    resource.generate_outputs()
+    settings.mappings[module.mapping_key].update(
+        {resource.logical_name: resource.mappings}
+    )
+
+
 class XStack(ComposeXStack):
     """
     Class to represent the root for EFS
@@ -155,9 +202,9 @@ def __init__(
                 setattr(self, "DeletionPolicy", module.module_deletion_policy)
         else:
             self.is_void = True
-        if module.lookup_resources:
-            warnings.warn(
-                f"{module.res_key} - Lookup not supported. You can only create new resources at the moment"
-            )
+        if module.lookup_resources and module.mapping_key not in settings.mappings:
+            settings.mappings[module.mapping_key] = {}
+        for resource in module.lookup_resources:
+            lookup_resource(module, resource, settings)
         for resource in module.resources_list:
             resource.stack = self