From 2d04b3b7c587fbeb28a4dec73ae2d20ff8be01f9 Mon Sep 17 00:00:00 2001 From: John Preston <1236150+JohnPreston@users.noreply.github.com> Date: Mon, 13 Nov 2023 07:52:02 +0000 Subject: [PATCH] Fix elbv2 conditions without setting access (#702) --- .../elbv2/elbv2_stack/elbv2_listener.py | 30 ++++++++++------- ecs_composex/elbv2/elbv2_stack/helpers.py | 33 ++++++++++--------- ecs_composex/elbv2/x-elbv2.spec.json | 12 +++++++ 3 files changed, 48 insertions(+), 27 deletions(-) diff --git a/ecs_composex/elbv2/elbv2_stack/elbv2_listener.py b/ecs_composex/elbv2/elbv2_stack/elbv2_listener.py index fca276ae..1923d768 100644 --- a/ecs_composex/elbv2/elbv2_stack/elbv2_listener.py +++ b/ecs_composex/elbv2/elbv2_stack/elbv2_listener.py @@ -24,11 +24,13 @@ LISTENER_TARGET_RE, add_acm_certs_arn, define_actions, + define_listener_rules_actions, handle_default_actions, handle_non_default_services, import_cognito_pool, import_new_acm_certs, map_service_target, + tea_pot, ) from ecs_composex.resources_import import import_record_properties @@ -92,6 +94,7 @@ def define_default_actions(self, lb: Elbv2, template): If not defined, and there is more than one service, it will fail. If not defined and there is only one service defined, it will skip """ + rules: list = [] if not self.default_actions and not self.services: warnings.warn( f"{self.name} - There are no actions defined or services for listener {self.title}. Skipping" @@ -100,10 +103,15 @@ def define_default_actions(self, lb: Elbv2, template): if self.default_actions: handle_default_actions(self) elif not self.default_actions and self.services and len(self.services) == 1: - LOG.info( - f"{self.title} has no defined DefaultActions and only 1 service. Default all to service." - ) - self.DefaultActions = define_actions(self, self.services[0]) + if not keyisset("Conditions", self.services[0]): + LOG.info( + f"{self.title} has no defined DefaultActions and only 1 service " + "without Conditions. Setting Listener DefaultActions to service." + ) + self.DefaultActions = define_actions(self, self.services[0]) + else: + self.DefaultActions = [tea_pot(True)] + rules = define_listener_rules_actions(self, self.services) elif lb.is_nlb() and self.services and len(self.services) > 1: raise ValueError( f"{lb.module.res_key}.{lb.name} - Listener {self.def_port}" @@ -116,15 +124,15 @@ def define_default_actions(self, lb: Elbv2, template): "If one of the access path is / it will be used as default" ) rules = handle_non_default_services(self) - if rules and lb.is_alb(): - for rule in rules: - template.add_resource(rule) - else: - LOG.warning( - f"{lb.module.res_key}.{lb.name} - LB is NLB. Can't assign Listener Rules." - ) else: raise ValueError(f"Failed to determine any default action for {self.title}") + if rules and lb.is_alb(): + for rule in rules: + template.add_resource(rule) + else: + LOG.warning( + f"{lb.module.res_key}.{lb.name} - LB is NLB. Can't assign Listener Rules." + ) def handle_cognito_pools(self, settings, listener_stack): """ diff --git a/ecs_composex/elbv2/elbv2_stack/helpers.py b/ecs_composex/elbv2/elbv2_stack/helpers.py index 9a90ed31..c18df094 100644 --- a/ecs_composex/elbv2/elbv2_stack/helpers.py +++ b/ecs_composex/elbv2/elbv2_stack/helpers.py @@ -16,7 +16,7 @@ from copy import deepcopy from json import dumps -from compose_x_common.compose_x_common import keyisset +from compose_x_common.compose_x_common import keyisset, set_else_none from troposphere import AWS_NO_VALUE, FindInMap, Ref from troposphere.elasticloadbalancingv2 import ( Action, @@ -198,9 +198,9 @@ def handle_predefined_redirects(listener: ComposeListener, action_name) -> None: f"Redirect {action_name} is not a valid pre-defined setting. Valid values", [r[0] for r in predefined_redirects], ) - for redirect in predefined_redirects: - if action_name == redirect[0]: - action = redirect[1]() + for redirect_key, redirect_function in predefined_redirects: + if action_name == redirect_key: + action = redirect_function() listener.DefaultActions.insert(0, action) @@ -276,7 +276,7 @@ def handle_string_condition_format(access_string) -> list: raise ValueError(f"Could not understand what the access is for {access_string}") -def define_target_conditions(definition) -> list: +def define_target_conditions(definition: dict) -> list: """ Function to create the conditions for forward to target @@ -285,11 +285,16 @@ def define_target_conditions(definition) -> list: :rtype: list """ conditions = [] - if keyisset("Conditions", definition) and isinstance( - definition["Conditions"], list - ): + user_defined_conditions = set_else_none("Conditions", definition, []) + if user_defined_conditions: + if not isinstance(user_defined_conditions, list): + raise TypeError( + "Conditions must be a list. Got {}".format( + type(user_defined_conditions) + ) + ) conditions = import_record_properties( - {"Conditions": definition["Conditions"]}, + {"Conditions": user_defined_conditions}, ListenerRule, set_to_novalue=False, ignore_missing_required=True, @@ -370,13 +375,11 @@ def define_actions(listener, target_def, rule_actions: bool = False) -> list: return actions -def define_listener_rules_actions(listener, left_services) -> list: +def define_listener_rules_actions( + listener: ComposeListener, left_services: list +) -> list[ListenerRule]: """ Function to identify the Target definition and create the resulting rule appropriately. - - :param listener: - :param list left_services: - :return: The action to add or action list for default target """ rules = [] for count, service_def in enumerate(left_services): @@ -528,8 +531,6 @@ def import_cognito_pool(src_name, settings: ComposeXSettings, listener_stack): raise LookupError( f"There is no {COGNITO_KEY} defined in your docker-compose files" ) - print("Cognito SRC Name", src_name) - print(settings.x_resources) pools = [ res for res in settings.x_resources diff --git a/ecs_composex/elbv2/x-elbv2.spec.json b/ecs_composex/elbv2/x-elbv2.spec.json index 7a6acd0f..597c7a6f 100644 --- a/ecs_composex/elbv2/x-elbv2.spec.json +++ b/ecs_composex/elbv2/x-elbv2.spec.json @@ -361,6 +361,18 @@ }, "Listener": { "type": "object", + "oneOf": [ + { + "required": [ + "DefaultActions" + ] + }, + { + "required": [ + "Targets" + ] + } + ], "properties": { "Port": { "$ref": "#/definitions/Port"