Services security groups in own stack (#750)

Author: JohnPreston

ecs_composex/ecs/ecs_family/__init__.py

@@ -304,9 +304,10 @@ def generate_outputs(self):
             self.outputs.append(
                 CfnOutput(
                     f"{self.logical_name}GroupId",
-                    Value=GetAtt(self.service_networking.security_group, "GroupId"),
+                    Value=Ref(self.service_networking.security_group.parameter.title),
                 )
             )
+        if ecs_params.SERVICE_SUBNETS.title in self.stack.stack_template.parameters:
             self.outputs.append(
                 CfnOutput(
                     ecs_params.SERVICE_SUBNETS.title,
@@ -423,7 +424,6 @@ def init_network_settings(
         """
         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, families_sg_stack)
         self.finalize_services_networking_settings(settings)
@@ -434,15 +434,14 @@ def init_network_settings(
                 self.stack.set_vpc_params_from_vpc_lookup(vpc_stack, settings)
             else:
                 self.stack.set_vpc_parameters_from_vpc_stack(vpc_stack, settings)
-            add_security_group(self)
             self.service_networking.ingress.set_aws_sources_ingress(
                 settings,
                 self.logical_name,
-                GetAtt(self.service_networking.security_group, "GroupId"),
+                Ref(self.service_networking.security_group.parameter.title),
             )
             self.service_networking.ingress.set_ext_sources_ingress(
                 self.logical_name,
-                GetAtt(self.service_networking.security_group, "GroupId"),
+                Ref(self.service_networking.security_group.parameter.title),
             )
             self.service_networking.ingress.associate_aws_ingress_rules(self.template)
             self.service_networking.ingress.associate_ext_ingress_rules(self.template)

ecs_composex/ecs/service_networking/__init__.py

@@ -17,6 +17,7 @@
         ServiceSecurityGroup,
     )
     from ecs_composex.common.settings import ComposeXSettings
+    from troposphere.ecs import ServiceConnectConfiguration
 
 from itertools import chain
 
@@ -76,11 +77,10 @@ def __init__(self, family: ComposeFamily, families_sg_stack: EcsIngressStack):
         self.merge_services_ports()
         self.merge_networks()
         self.definition = merge_family_services_networking(family)
-        self._security_group = None
-        self.inter_services_sg: ServiceSecurityGroup = (
-            families_sg_stack.services_mappings[family.name]
-        )
-        self.extra_security_groups = [self.inter_services_sg.parameter]
+        self.security_group: ServiceSecurityGroup = families_sg_stack.services_mappings[
+            family.name
+        ]
+        self.extra_security_groups = [self.security_group.parameter]
         self._subnets = Ref(APP_SUBNETS)
         self.cloudmap_config = (
             merge_cloudmap_settings(family, self.ports) if self.ports else {}
@@ -139,7 +139,7 @@ def eip_assign(self):
 
     @property
     def security_groups(self) -> list:
-        groups = [Ref(self.security_group)]
+        groups = [Ref(self.security_group.parameter.title)]
         for extra_group in self.extra_security_groups:
             if (
                 isinstance(extra_group, SecurityGroup)
@@ -153,23 +153,6 @@ def security_groups(self) -> list:
                 groups.append(extra_group)
         return groups
 
-    @property
-    def security_group(self):
-        return self._security_group
-
-    @security_group.setter
-    def security_group(self, value):
-        if isinstance(value, SecurityGroup):
-            self._security_group = value
-        else:
-            raise TypeError(
-                "Service security group must be",
-                SecurityGroup,
-                "Got",
-                value,
-                type(value),
-            )
-
     @property
     def network_mode(self):
         """
@@ -270,11 +253,11 @@ def add_self_ingress(self) -> None:
                     FromPort=target_port,
                     ToPort=target_port,
                     IpProtocol=port["protocol"],
-                    GroupId=GetAtt(
-                        self.family.service_networking.security_group, "GroupId"
+                    GroupId=Ref(
+                        self.family.service_networking.security_group.parameter
                     ),
-                    SourceSecurityGroupId=GetAtt(
-                        self.family.service_networking.security_group, "GroupId"
+                    SourceSecurityGroupId=Ref(
+                        self.family.service_networking.security_group.parameter
                     ),
                     SourceSecurityGroupOwnerId=Ref(AWS_ACCOUNT_ID),
                     Description=Sub(
@@ -299,9 +282,7 @@ def add_lb_ingress(self, lb_name, lb_sg_ref) -> None:
                 "FromPort": port["target"],
                 "ToPort": port["target"],
                 "IpProtocol": port["protocol"],
-                "GroupId": GetAtt(
-                    self.family.service_networking.security_group, "GroupId"
-                ),
+                "GroupId": Ref(self.security_group.parameter.title),
                 "SourceSecurityGroupOwnerId": Ref(AWS_ACCOUNT_ID),
                 "Description": Sub(
                     f"From ELB {lb_name} to ${{{SERVICE_NAME.title}}} on port {port['target']}"

ecs_composex/ecs/service_networking/helpers.py

@@ -1,50 +1,23 @@
 #  SPDX-License-Identifier: MPL-2.0
 #  Copyright 2020-2022 John Mille <[email protected]>
 
-from troposphere import Ref, StackName, Sub, Tags
-from troposphere.ec2 import SecurityGroup
+from __future__ import annotations
 
-from ecs_composex.common.cfn_conditions import define_stack_name
-from ecs_composex.common.logging import LOG
-from ecs_composex.common.troposphere_tools import add_resource
-from ecs_composex.ecs.ecs_params import SERVICE_NAME, SG_T
-from ecs_composex.vpc.vpc_params import VPC_ID
+from typing import TYPE_CHECKING
 
+if TYPE_CHECKING:
+    from ecs_composex.ecs.ecs_family import ComposeFamily
+    from ecs_composex.common.settings import ComposeXSettings
 
-def add_security_group(family) -> None:
-    """
-    Creates a new EC2 SecurityGroup and assigns to ecs_service.network_settings
-    Adds the security group to the family template resources.
+from troposphere import Ref
 
-    :param ecs_composex.ecs.ecs_family.ComposeFamily family:
-    """
-    family.service_networking.security_group = SecurityGroup(
-        SG_T,
-        GroupDescription=Sub(
-            f"SG for ${{{SERVICE_NAME.title}}} - ${{STACK_NAME}}",
-            STACK_NAME=define_stack_name(),
-        ),
-        Tags=Tags(
-            {
-                "Name": Sub(
-                    f"${{{SERVICE_NAME.title}}}-${{STACK_NAME}}",
-                    STACK_NAME=define_stack_name(),
-                ),
-                "StackName": StackName,
-                "MicroserviceName": Ref(SERVICE_NAME),
-            }
-        ),
-        VpcId=Ref(VPC_ID),
-    )
-    add_resource(family.template, family.service_networking.security_group)
+from ecs_composex.common.logging import LOG
 
 
-def update_family_subnets(family, settings) -> None:
+def update_family_subnets(family: ComposeFamily, settings: ComposeXSettings) -> None:
     """
-    Method to update the stack parameters
-
-    :param ecs_composex.ecs.ecs_family.ComposeFamily family:
-    :param ecs_composex.common.settings.ComposeXSettings settings:
+    Update the stack parameters of the family stack AppSubnets Parameter value to the one matching with
+    networks.x-vpc and networks.[]
     """
     network_names = list(family.service_networking.networks.keys())
     for network in settings.networks:
@@ -63,7 +36,11 @@ def update_family_subnets(family, settings) -> None:
         )
 
 
-def set_family_hostname(family):
+def set_family_hostname(family: ComposeFamily):
+    """
+    Sets the hostname to use for the Family in Cloudmap.
+    If it has been set on more than one service container, it uses the first one.
+    """
     svcs_hostnames = any(svc.family_hostname for svc in family.services)
     if not svcs_hostnames or not family.family_hostname:
         LOG.debug(

ecs_composex/ecs/service_networking/ingress_helpers.py

@@ -216,11 +216,11 @@ def add_service_to_service_ingress_rules(
                 SecurityGroupIngress(
                     ingress_title,
                     SourceSecurityGroupId=GetAtt(
-                        _service.family.service_networking.inter_services_sg.cfn_resource,
+                        _service.family.service_networking.security_group.cfn_resource,
                         "GroupId",
                     ),
                     GroupId=GetAtt(
-                        dst_family.service_networking.inter_services_sg.cfn_resource,
+                        dst_family.service_networking.security_group.cfn_resource,
                         "GroupId",
                     ),
                     **common_args,

ecs_composex/ingress_settings.py

@@ -5,6 +5,14 @@
 Module to help with defining the network settings for the ECS Service based on the family services definitions.
 """
 
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from troposphere import AWSHelperFn
+    from ecs_composex.common.settings import ComposeXSettings
+
 import re
 from copy import deepcopy
 from ipaddress import IPv4Interface
@@ -153,14 +161,8 @@ def set_service_ports(ports):
     return service_ports
 
 
-def lookup_security_group(settings, lookup):
-    """
-    Function to fetch the security group ID based on lookup details
-
-    :param ecs_composex.common.settings.ComposeXSettings settings:
-    :param lookup:
-    :return:
-    """
+def lookup_security_group(settings: ComposeXSettings, lookup: dict | list) -> str:
+    """Function to fetch the security group ID based on lookup details"""
     sg_re = re.compile(
         r"^arn:aws(?:-[a-z]+)?:ec2:[a-z0-9-]+:\d{12}:security-group/([\S]+)$"
     )
@@ -239,14 +241,48 @@ def __init__(self, definition, ports):
     def __repr__(self):
         return dumps(self.definition, indent=2)
 
-    def set_aws_sources_ingress(self, settings, destination_title, sg_ref) -> None:
+    def handle_security_group_source(
+        self,
+        source,
+        common_args: dict,
+        destination_title: str,
+        target_port: int,
+        settings,
+    ) -> None:
         """
-        Method to define AWS Sources ingresses
-
-        :param settings:
-        :param destination_title:
-        :param sg_ref:
+        Method to handle SecurityGroup sources
+        It updates the list of AWS sources ingress rules that will later be added to the stack template of the family
         """
+        if keyisset("Id", source):
+            sg_id = source["Id"]
+        elif keyisset("Lookup", source):
+            sg_id = lookup_security_group(settings, source["Lookup"])
+        else:
+            raise KeyError(
+                "Information missing to identify the SecurityGroup. Requires either Id or Lookup"
+            )
+        common_args.update(
+            {
+                "Description": Sub(
+                    f"From {sg_id} to {destination_title} on port {target_port}"
+                )
+            }
+        )
+        self.aws_ingress_rules.append(
+            SecurityGroupIngress(
+                f"From{NONALPHANUM.sub('', sg_id)}ToServiceOn{target_port}",
+                SourceSecurityGroupId=sg_id,
+                SourceSecurityGroupOwnerId=set_else_none(
+                    "AccountOwner", source, Ref(AWS_ACCOUNT_ID)
+                ),
+                **common_args,
+            )
+        )
+
+    def set_aws_sources_ingress(
+        self, settings: ComposeXSettings, destination_title: str, sg_ref: AWSHelperFn
+    ) -> None:
+        """Method to define AWS Sources ingresses"""
         for source in self.aws_sources:
             for port in self.ports:
                 if (
@@ -269,30 +305,8 @@ def set_aws_sources_ingress(self, settings, destination_title, sg_ref) -> None:
                     "GroupId": sg_ref,
                 }
                 if source["Type"] == "SecurityGroup":
-                    if keyisset("Id", source):
-                        sg_id = source["Id"]
-                    elif keyisset("Lookup", source):
-                        sg_id = lookup_security_group(settings, source["Lookup"])
-                    else:
-                        raise KeyError(
-                            "Information missing to identify the SecurityGroup. Requires either Id or Lookup"
-                        )
-                    common_args.update(
-                        {
-                            "Description": Sub(
-                                f"From {sg_id} to {destination_title} on port {target_port}"
-                            )
-                        }
-                    )
-                    self.aws_ingress_rules.append(
-                        SecurityGroupIngress(
-                            f"From{NONALPHANUM.sub('', sg_id)}ToServiceOn{target_port}",
-                            SourceSecurityGroupId=sg_id,
-                            SourceSecurityGroupOwnerId=set_else_none(
-                                "AccountOwner", source, Ref(AWS_ACCOUNT_ID)
-                            ),
-                            **common_args,
-                        )
+                    self.handle_security_group_source(
+                        source, common_args, destination_title, target_port, settings
                     )
                 elif source["Type"] == "PrefixList":
                     self.aws_ingress_rules.append(
@@ -304,7 +318,7 @@ def set_aws_sources_ingress(self, settings, destination_title, sg_ref) -> None:
                     )
 
     def create_ext_sources_ingress_rule(
-        self, destination_title, allowed_source, security_group, **props
+        self, destination_title, allowed_source, security_group: AWSHelperFn, **props
     ) -> None:
         """
         Creates the Security Ingress rule for a CIDR based rule

ecs_composex/rds_resources_settings.py

@@ -17,6 +17,7 @@
         DatabaseXResource,
     )
     from ecs_composex.common.stacks import ComposeXStack
+    from ecs_composex.ecs.ecs_stack import ServiceStack
 
 from botocore.exceptions import ClientError
 from compose_x_common.aws import get_account_id
@@ -380,25 +381,20 @@ def add_secret_to_container(db, secret_import, service, target):
         extend_container_secrets(service.container_definition, db_secret)
 
 
-def add_security_group_ingress(service_stack: ComposeXStack, db_name: str, sg_id, port):
+def add_security_group_ingress(family: ComposeFamily, db_name: str, sg_id, port):
     """
-    Function to add a SecurityGroupIngress rule into the ECS Service template
-
-    :param ecs_composex.ecs.ServicesStack service_stack: The root stack for the services
-    :param str db_name: the name of the database to use for imports
-    :param sg_id: The security group Id to use for ingress. DB Security group, not service's
-    :param port: The port for Ingress to the DB.
+    Add a SecurityGroupIngress rule into the ECS Service template, allowing the Service access to the DB.
     """
     add_resource(
-        service_stack.stack_template,
+        family.stack.stack_template,
         SecurityGroupIngress(
-            f"AllowFrom{service_stack.title}to{db_name}",
+            f"AllowFrom{family.stack.title}to{db_name}",
             GroupId=sg_id,
             FromPort=port,
             ToPort=port,
-            Description=Sub(f"Allow FROM {service_stack.title} TO {db_name}"),
-            SourceSecurityGroupId=GetAtt(
-                service_stack.stack_template.resources[SG_T], "GroupId"
+            Description=Sub(f"Allow FROM {family.stack.title} TO {db_name}"),
+            SourceSecurityGroupId=Ref(
+                family.service_networking.security_group.parameter.title
             ),
             SourceSecurityGroupOwnerId=Ref("AWS::AccountId"),
             IpProtocol="6",
@@ -553,7 +549,7 @@ def handle_import_dbs_to_services(
             use_task_role=grant_task_role_access,
         )
     add_security_group_ingress(
-        target[0].stack,
+        target[0],
         db.logical_name,
         sg_id=db.attributes_outputs[db.security_group_param]["ImportValue"],
         port=db.attributes_outputs[db.port_param]["ImportValue"],
@@ -604,7 +600,7 @@ def handle_new_tcp_resource(
             )
 
             add_security_group_ingress(
-                target[0].stack,
+                target[0],
                 resource.logical_name,
                 sg_id=Ref(sg_id["ImportParameter"]),
                 port=Ref(port_id["ImportParameter"]),