From e5db6cc3d40efcd41062ddc8261dd1a77dfc2d31 Mon Sep 17 00:00:00 2001 From: "John \"Preston\" Mille" Date: Sat, 30 Mar 2024 09:16:53 +0000 Subject: [PATCH] Security groups at root-stack level specifically for service-to-service --- ecs_composex/ecs/ecs_family/__init__.py | 4 +- ecs_composex/ecs/ecs_stack.py | 16 +- ecs_composex/ecs/helpers/__init__.py | 22 +-- .../ecs/service_networking/__init__.py | 11 +- .../ecs/service_networking/ingress_helpers.py | 174 +++++++----------- ecs_composex/ecs_composex.py | 13 +- ecs_composex/ecs_ingress/__init__.py | 8 + ecs_composex/ecs_ingress/ecs_ingress_stack.py | 98 ++++++++++ 8 files changed, 219 insertions(+), 127 deletions(-) create mode 100644 ecs_composex/ecs_ingress/__init__.py create mode 100644 ecs_composex/ecs_ingress/ecs_ingress_stack.py diff --git a/ecs_composex/ecs/ecs_family/__init__.py b/ecs_composex/ecs/ecs_family/__init__.py index f789bc73..180d134c 100644 --- a/ecs_composex/ecs/ecs_family/__init__.py +++ b/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") diff --git a/ecs_composex/ecs/ecs_stack.py b/ecs_composex/ecs/ecs_stack.py index a722ef6a..1123b402 100644 --- a/ecs_composex/ecs/ecs_stack.py +++ b/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) diff --git a/ecs_composex/ecs/helpers/__init__.py b/ecs_composex/ecs/helpers/__init__.py index 569392c3..51394246 100644 --- a/ecs_composex/ecs/helpers/__init__.py +++ b/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, ) diff --git a/ecs_composex/ecs/service_networking/__init__.py b/ecs_composex/ecs/service_networking/__init__.py index e095339f..4af0644a 100644 --- a/ecs_composex/ecs/service_networking/__init__.py +++ b/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 {} diff --git a/ecs_composex/ecs/service_networking/ingress_helpers.py b/ecs_composex/ecs/service_networking/ingress_helpers.py index 614fb493..bb0d6659 100644 --- a/ecs_composex/ecs/service_networking/ingress_helpers.py +++ b/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( diff --git a/ecs_composex/ecs_composex.py b/ecs_composex/ecs_composex.py index d00e210a..85cf17cc 100644 --- a/ecs_composex/ecs_composex.py +++ b/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( diff --git a/ecs_composex/ecs_ingress/__init__.py b/ecs_composex/ecs_ingress/__init__.py new file mode 100644 index 00000000..9eb829c1 --- /dev/null +++ b/ecs_composex/ecs_ingress/__init__.py @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: MPL-2.0 +# Copyright 2020-2022 John Mille + +""" +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. +""" diff --git a/ecs_composex/ecs_ingress/ecs_ingress_stack.py b/ecs_composex/ecs_ingress/ecs_ingress_stack.py new file mode 100644 index 00000000..b8c5aebe --- /dev/null +++ b/ecs_composex/ecs_ingress/ecs_ingress_stack.py @@ -0,0 +1,98 @@ +# SPDX-License-Identifier: MPL-2.0 +# Copyright 2024 John Mille + +""" + +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from ecs_composex.common.settings import ComposeXSettings + from ecs_composex.ecs.ecs_family import ComposeFamily + +from troposphere import FindInMap, GetAtt, Output, Ref, Sub, Tags +from troposphere.ec2 import SecurityGroup + +from ecs_composex.common.cfn_conditions import define_stack_name +from ecs_composex.common.cfn_params import Parameter +from ecs_composex.common.stacks import ComposeXStack +from ecs_composex.common.troposphere_tools import ( + add_outputs, + add_parameters, + add_resource, + build_template, +) +from ecs_composex.ecs.ecs_params import CLUSTER_NAME +from ecs_composex.vpc.vpc_params import VPC_ID +from ecs_composex.vpc.vpc_stack import XStack as VpcStack + + +class ServiceSecurityGroup: + + def __init__(self, family: ComposeFamily, sgs_stack: XStack): + self.family = family + self.stack: XStack = sgs_stack + cfn_resource = SecurityGroup( + f"{family.logical_name}SG", + GroupDescription=Sub( + f"SG for {family.logical_name} in ${{ROOT_STACK}}", + ROOT_STACK=define_stack_name(sgs_stack.stack_template), + ), + VpcId=Ref(VPC_ID), + Tags=Tags( + { + "Name": Sub( + f"${family.logical_name}-${{STACK_NAME}}", + STACK_NAME=define_stack_name(), + ), + "compose-x:family-name": family.name, + "compose-x:family-logical-name": family.logical_name, + } + ), + ) + self.cfn_resource = add_resource(sgs_stack.stack_template, cfn_resource) + self.output = Output( + self.cfn_resource.title, Value=GetAtt(self.cfn_resource, "GroupId") + ) + self.parameter = Parameter( + self.cfn_resource.title, + return_value="GroupId", + group_label="Networking", + label="Service to Service Security Group ID", + Type="AWS::EC2::SecurityGroup::Id", + ) + + +class XStack(ComposeXStack): + """ + Class to represent the IAM top stack + """ + + def __init__(self, name: str, settings: ComposeXSettings, **kwargs): + stack_template = build_template( + "Services SG for service-to-service communication" + ) + self.services_mappings: dict[str, ServiceSecurityGroup] = {} + add_parameters(stack_template, [CLUSTER_NAME, VPC_ID]) + super().__init__(name, stack_template, **kwargs) + + for family in settings.families.values(): + sg = ServiceSecurityGroup(family, self) + self.services_mappings[family.name] = sg + add_outputs(stack_template, [sg.output]) + + def update_vpc_settings(self, vpc_stack: VpcStack): + if vpc_stack.vpc_resource and ( + vpc_stack.vpc_resource.cfn_resource or vpc_stack.vpc_resource.mappings + ): + if vpc_stack.vpc_resource.cfn_resource: + self.Parameters[VPC_ID.title] = GetAtt( + vpc_stack.title, f"Outputs.{VPC_ID.title}" + ) + else: + self.Parameters.update( + {VPC_ID.title: FindInMap("Network", VPC_ID.title, VPC_ID.title)} + )