Security groups at root-stack level specifically for service-to-service (#747)

Author: JohnPreston

ecs_composex/ecs/ecs_family/__init__.py

@@ -417,14 +417,14 @@ def finalize_services_networking_settings(self, settings: ComposeXSettings) -> N
                 )
 
     def init_network_settings(
-        self, settings: ComposeXSettings, vpc_stack: ComposeXStack
+        self, settings: ComposeXSettings, vpc_stack: ComposeXStack, families_sg_stack
     ) -> None:
         """
         Once we have figured out the compute settings (EXTERNAL vs other)
         """
         from ecs_composex.ecs.service_networking.helpers import add_security_group
 
-        self.service_networking = ServiceNetworking(self)
+        self.service_networking = ServiceNetworking(self, families_sg_stack)
         self.finalize_services_networking_settings(settings)
         if self.service_compute.launch_type == "EXTERNAL":
             LOG.debug(f"{self.name} Ingress cannot be set (EXTERNAL mode). Skipping")

ecs_composex/ecs/ecs_stack.py

@@ -8,8 +8,9 @@
 if TYPE_CHECKING:
     from ecs_composex.ecs.ecs_family import ComposeFamily
     from ecs_composex.common.settings import ComposeXSettings
+    from ecs_composex.ecs_ingress.ecs_ingress_stack import XStack as EcsIngressStack
 
-from troposphere import FindInMap, Ref
+from troposphere import FindInMap, GetAtt, Ref
 
 from ecs_composex.common.cfn_params import ROOT_STACK_NAME, ROOT_STACK_NAME_T
 from ecs_composex.common.logging import LOG
@@ -88,12 +89,12 @@ def handle_families_dependencies(
                 )
 
 
-def add_compose_families(settings: ComposeXSettings) -> None:
+def add_compose_families(
+    settings: ComposeXSettings, families_sg_stack: EcsIngressStack
+) -> None:
     """
     Using existing ComposeFamily in settings, creates the ServiceStack
     and template
-
-    :param ecs_composex.common.settings.ComposeXSettings settings:
     """
     for family_name, family in settings.families.items():
         family.init_family()
@@ -105,6 +106,7 @@ def add_compose_families(settings: ComposeXSettings) -> None:
                 family.iam_manager.task_role.name_param,
                 family.iam_manager.exec_role.arn_param,
                 family.iam_manager.exec_role.name_param,
+                families_sg_stack.services_mappings[family.name].parameter,
             ],
         )
         family.stack.Parameters.update(
@@ -118,6 +120,12 @@ def add_compose_families(settings: ComposeXSettings) -> None:
                 family.iam_manager.exec_role.arn_param.title: family.iam_manager.exec_role.output_arn,
                 family.iam_manager.exec_role.name_param.title: family.iam_manager.exec_role.output_name,
                 ecs_params.SERVICE_HOSTNAME.title: family.family_hostname,
+                families_sg_stack.services_mappings[
+                    family.name
+                ].parameter.title: GetAtt(
+                    families_sg_stack.services_mappings[family.name].stack.title,
+                    f"Outputs.{families_sg_stack.services_mappings[family.name].parameter.title}",
+                ),
             }
         )
         family.template.metadata.update(metadata)

ecs_composex/ecs/helpers/__init__.py

@@ -8,6 +8,7 @@
 if TYPE_CHECKING:
     from ecs_composex.common.settings import ComposeXSettings
     from ecs_composex.ecs.ecs_family import ComposeFamily
+    from ecs_composex.ecs_ingress.ecs_ingress_stack import XStack as EcsIngressStack
 
 from ecs_composex.common.stacks import ComposeXStack
 
@@ -24,24 +25,23 @@ def add_iam_dependency(iam_stack: ComposeXStack, family: ComposeFamily):
 
 
 def handle_families_cross_dependencies(
-    settings: ComposeXSettings, root_stack: ComposeXStack
+    settings: ComposeXSettings,
+    families_sg_stack: EcsIngressStack,
 ):
     from ecs_composex.ecs.ecs_family import ServiceStack
     from ecs_composex.ecs.service_networking.ingress_helpers import (
         set_compose_services_ingress,
     )
 
-    families_stacks = [
-        family
-        for family in root_stack.stack_template.resources
-        if (
-            family in settings.families
-            and isinstance(settings.families[family].stack, ServiceStack)
-        )
-    ]
-    for family in families_stacks:
+    eval_families: list[ComposeFamily] = []
+    for _family in settings.families.values():
+        if isinstance(_family.stack, ServiceStack):
+            eval_families.append(_family)
+    for _dst_family in eval_families:
         set_compose_services_ingress(
-            root_stack, settings.families[family], families_stacks, settings
+            _dst_family,
+            families_sg_stack,
+            settings,
         )
 
 

ecs_composex/ecs/service_networking/__init__.py

@@ -12,6 +12,10 @@
 if TYPE_CHECKING:
     from ecs_composex.ecs.ecs_family import ComposeFamily
     from ecs_composex.cloudmap.cloudmap_ecs import EcsDiscoveryService
+    from ecs_composex.ecs_ingress.ecs_ingress_stack import (
+        XStack as EcsIngressStack,
+        ServiceSecurityGroup,
+    )
 
 from itertools import chain
 
@@ -51,7 +55,7 @@ class ServiceNetworking:
 
     self_key = "Myself"
 
-    def __init__(self, family: ComposeFamily):
+    def __init__(self, family: ComposeFamily, families_sg_stack: EcsIngressStack):
         """
         Initialize network settings for the family ServiceConfig
 
@@ -71,7 +75,10 @@ def __init__(self, family: ComposeFamily):
         self.merge_networks()
         self.definition = merge_family_services_networking(family)
         self._security_group = None
-        self.extra_security_groups = []
+        self.inter_services_sg: ServiceSecurityGroup = (
+            families_sg_stack.services_mappings[family.name]
+        )
+        self.extra_security_groups = [self.inter_services_sg.parameter]
         self._subnets = Ref(APP_SUBNETS)
         self.cloudmap_config = (
             merge_cloudmap_settings(family, self.ports) if self.ports else {}

ecs_composex/ecs/service_networking/ingress_helpers.py

@@ -9,8 +9,10 @@
 
 if TYPE_CHECKING:
     from ecs_composex.ecs.ecs_family import ComposeFamily
+    from ecs_composex.compose.compose_services import ComposeService
     from ecs_composex.common.settings import ComposeXSettings
     from ecs_composex.common.stacks import ComposeXStack
+    from ecs_composex.ecs_ingress.ecs_ingress_stack import XStack as EcsIngressStack
 
 from json import dumps
 
@@ -21,7 +23,7 @@
 from ecs_composex.cloudmap.cloudmap_params import RES_KEY as CLOUDMAP_KEY
 from ecs_composex.common.cfn_params import Parameter
 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 SERVICE_NAME
 from ecs_composex.ingress_settings import Ingress
 from ecs_composex.vpc.vpc_params import SG_ID_TYPE
@@ -142,120 +144,82 @@ def merge_family_services_networking(family: ComposeFamily) -> dict:
     return network_config
 
 
-def add_independent_rules(
-    dst_family: ComposeFamily, service_name: str, root_stack: ComposeXStack
-) -> None:
-    """
-    Adds security groups rules in the root stack as both services need to be created (with their SG)
-    before the ingress rule can be defined.
-
-    :param dst_family:
-    :param service_name:
-    :param root_stack:
-    :return:
-    """
-    src_service_stack = root_stack.stack_template.resources[service_name]
-    for port in dst_family.service_networking.ports:
-        target_port = set_else_none(
-            "published", port, alt_value=set_else_none("target", port, None)
-        )
-        if target_port is None:
-            raise ValueError(
-                "Wrong port definition value for security group ingress", port
-            )
-        ingress_rule = SecurityGroupIngress(
-            f"From{src_service_stack.title}To{dst_family.logical_name}On{target_port}",
-            FromPort=target_port,
-            ToPort=target_port,
-            IpProtocol=port["protocol"],
-            Description=Sub(
-                f"From {src_service_stack.title} to {dst_family.logical_name}"
-                f" on port {target_port}/{port['protocol']}"
-            ),
-            GroupId=GetAtt(
-                dst_family.stack.title,
-                f"Outputs.{dst_family.logical_name}GroupId",
-            ),
-            SourceSecurityGroupId=GetAtt(
-                src_service_stack.title,
-                f"Outputs.{src_service_stack.title}GroupId",
-            ),
-            SourceSecurityGroupOwnerId=Ref(AWS_ACCOUNT_ID),
-        )
-        if ingress_rule.title not in root_stack.stack_template.resources:
-            root_stack.stack_template.add_resource(ingress_rule)
-
-
-def add_dependant_ingress_rules(
-    dst_family: ComposeFamily, dst_family_sg_param: Parameter, src_family: ComposeFamily
-) -> None:
-    for port in dst_family.service_networking.ports:
-        target_port = set_else_none(
-            "published", port, alt_value=set_else_none("target", port, None)
-        )
-        if target_port is None:
-            raise ValueError(
-                "Wrong port definition value for security group ingress", port
-            )
-        common_args = {
-            "FromPort": target_port,
-            "ToPort": target_port,
-            "IpProtocol": port["protocol"],
-            "SourceSecurityGroupOwnerId": Ref(AWS_ACCOUNT_ID),
-            "Description": Sub(
-                f"From ${{{SERVICE_NAME.title}}} to {dst_family.stack.title} on port {target_port}"
-            ),
-        }
-        src_family.template.add_resource(
-            SecurityGroupIngress(
-                f"From{src_family.logical_name}To{dst_family.stack.title}On{target_port}",
-                SourceSecurityGroupId=GetAtt(
-                    src_family.service_networking.security_group, "GroupId"
-                ),
-                GroupId=Ref(dst_family_sg_param),
-                **common_args,
-            )
-        )
-
-
 def set_compose_services_ingress(
-    root_stack, dst_family: ComposeFamily, families: list, settings: ComposeXSettings
+    dst_family: ComposeFamily,
+    families_sg_stack: EcsIngressStack,
+    settings: ComposeXSettings,
 ) -> None:
     """
     Function to crate SG Ingress between two families / services.
     Presently, the ingress rules are set after all services have been created
-
-    :param ecs_composex.common.stacks.ComposeXStack root_stack:
-    :param ecs_composex.ecs.ecs_family.ComposeFamily dst_family:
-    :param list families: The list of family names.
-    :param ecs_composex.common.settings.ComposeXSettings settings:
     """
-    for service in dst_family.service_networking.ingress.services:
-        service_name = service["Name"]
-        if service_name not in families:
-            raise KeyError(
-                f"The service {service_name} is not among the services created together. Valid services are",
-                families,
+    target_family_services: list[ComposeService] = []
+    for _target_service_def in dst_family.service_networking.ingress.services:
+        service_name = _target_service_def["Name"]
+        for _service in settings.services:
+            if service_name != _service.name:
+                continue
+            if _service.family == dst_family:
+                continue
+            target_family_services.append(_service)
+    add_service_to_service_ingress_rules(
+        dst_family, target_family_services, families_sg_stack
+    )
+
+
+def add_service_to_service_ingress_rules(
+    dst_family: ComposeFamily,
+    target_family_services: list[ComposeService],
+    families_sg_stack: EcsIngressStack,
+):
+    """
+    For each identified service that wants to access the `dst_family` services
+    For each port of the `dst_family`
+    Create an SG Ingress rule that allows service-to-service communication
+    """
+    for _service in target_family_services:
+        if families_sg_stack.title not in _service.family.stack.DependsOn:
+            _service.family.stack.DependsOn.append(families_sg_stack.title)
+        for _service_port_def in dst_family.service_networking.ports:
+            target_port = set_else_none(
+                "target",
+                _service_port_def,
+                set_else_none("published", _service_port_def, None),
             )
-        if not keypresent("DependsOn", service):
-            add_independent_rules(dst_family, service_name, root_stack)
-        else:
-            src_family = settings.families[service_name]
-            if dst_family.stack.title not in src_family.stack.DependsOn:
-                src_family.stack.DependsOn.append(dst_family.stack.title)
-            dst_family_sg_param = Parameter(
-                f"{dst_family.stack.title}GroupId", Type=SG_ID_TYPE
+            if target_port is None:
+                raise ValueError(
+                    "Wrong port definition value for security group ingress",
+                    _service_port_def,
+                )
+            common_args = {
+                "FromPort": target_port,
+                "ToPort": target_port,
+                "IpProtocol": _service_port_def["protocol"],
+                "SourceSecurityGroupOwnerId": Ref(AWS_ACCOUNT_ID),
+                "Description": Sub(
+                    f"From ${_service.family.name} to {dst_family.name} "
+                    f"on port {target_port}/{_service_port_def['protocol']}"
+                ),
+            }
+            ingress_title: str = (
+                f"From{_service.family.logical_name}To{dst_family.logical_name}"
+                f"On{target_port}{_service_port_def['protocol'].title()}"
             )
-            add_parameters(src_family.template, [dst_family_sg_param])
-            src_family.stack.Parameters.update(
-                {
-                    dst_family_sg_param.title: GetAtt(
-                        dst_family.stack.title,
-                        f"Outputs.{dst_family.logical_name}GroupId",
+            add_resource(
+                families_sg_stack.stack_template,
+                SecurityGroupIngress(
+                    ingress_title,
+                    SourceSecurityGroupId=GetAtt(
+                        _service.family.service_networking.inter_services_sg.cfn_resource,
+                        "GroupId",
                     ),
-                }
+                    GroupId=GetAtt(
+                        dst_family.service_networking.inter_services_sg.cfn_resource,
+                        "GroupId",
+                    ),
+                    **common_args,
+                ),
             )
-            add_dependant_ingress_rules(dst_family, dst_family_sg_param, src_family)
 
 
 def handle_str_cloudmap_config(

ecs_composex/ecs_composex.py

@@ -41,6 +41,7 @@
 )
 from ecs_composex.ecs_cluster import add_ecs_cluster
 from ecs_composex.ecs_cluster.helpers import set_ecs_cluster_identifier
+from ecs_composex.ecs_ingress.ecs_ingress_stack import XStack as ServicesIngressStack
 from ecs_composex.iam.iam_stack import XStack as IamStack
 from ecs_composex.mods_manager import ModManager
 from ecs_composex.resource_settings import map_resource_return_value_to_services_command
@@ -248,8 +249,13 @@ def generate_full_template(settings: ComposeXSettings):
     iam_stack = add_resource(
         settings.root_stack.stack_template, IamStack("iam", settings)
     )
+    families_sg_stack = add_resource(
+        settings.root_stack.stack_template,
+        ServicesIngressStack("ServicesNetworking", settings),
+    )
+
     add_x_resources(settings)
-    add_compose_families(settings)
+    add_compose_families(settings, families_sg_stack)
     if "x-vpc" not in settings.mod_manager.modules:
         vpc_module = settings.mod_manager.load_module("x-vpc", {})
     else:
@@ -264,9 +270,9 @@ def generate_full_template(settings: ComposeXSettings):
     x_cloud_lookup_and_new_vpc(settings, vpc_stack)
 
     for family in settings.families.values():
-        family.init_network_settings(settings, vpc_stack)
+        family.init_network_settings(settings, vpc_stack, families_sg_stack)
 
-    handle_families_cross_dependencies(settings, settings.root_stack)
+    handle_families_cross_dependencies(settings, families_sg_stack)
     update_network_resources_vpc_config(settings, vpc_stack)
     set_families_ecs_service(settings)
 
@@ -304,6 +310,7 @@ def generate_full_template(settings: ComposeXSettings):
     set_ecs_cluster_identifier(settings.root_stack, settings)
     add_all_tags(settings.root_stack.stack_template, settings)
     set_all_mappings_to_root_stack(settings)
+    families_sg_stack.update_vpc_settings(vpc_stack)
 
     for resource in settings.x_resources:
         if hasattr(resource, "post_processing") and hasattr(

ecs_composex/ecs_ingress/__init__.py

@@ -0,0 +1,8 @@
+#  SPDX-License-Identifier: MPL-2.0
+#  Copyright 2020-2022 John Mille <john@compose-x.io>
+
+"""
+Root stack to store and manage the security groups of the services
+Having the security groups created before the services stacks allows to define service to service communication
+to be defined before the services are deployed.
+"""