From 3f5408c9d0292260ec772acd82f757a15919d259 Mon Sep 17 00:00:00 2001 From: Sahil Shah <53784576+sahilshah6196@users.noreply.github.com> Date: Wed, 9 Jun 2021 13:41:18 -0400 Subject: [PATCH] Adding support for `ForwardConfig` property in `ListernRule` in CloudFormation (#3993) * Add ssm parsing support for cloudformation stacks * Start adding unit tests for ssm parameter parsing * Add tests for code update * Add tests to parse ssm parameters code * Fix black lint errors * Fix bug. * Need to specify region_name * region needs to be same * Use ssm_backends[region] instead of ssm_backend * StringList -> string * Linting * check if servermode tests are on * Typo * Added support for ListenerRule. Will remove cruft * Pushing latest * Something works * Put back ripped out code * Save point. Incase I need more validations * Revert "Save point. Incase I need more validations" This reverts commit dac4953335dd9335eddb7a91a63667bc3c17104c. * Fixed validations and some refactor * Fix formatting * Linting * Cannot refactor if I have to fix all tests * Remove exceptions for now. Will do in another PR * Remove validations. Will add in next PR * Fix broken tests. Almost.: * Fix all tests. Some sneaky for now. * Python2 making me write bad code * OrderedDict.move_to_end() does not work in python2 * Linting * Add more checks to field in conditions later. * Unwnated change in FakeListener * Revert "Unwnated change in FakeListener" This reverts commit 962c2fdfd76fce999de9feccf1dd1c3ec48c459f. * Add back default listener rule * Linting fix * Fix priority sorting * Add cloudformation test for edge case * Add validation for ForwardConfig in Action of ListernRule CF * use not in * set the priority template correctly * Check for boolean in condition * One more check Co-authored-by: Bert Blommers --- moto/elbv2/models.py | 45 +++++++- moto/elbv2/responses.py | 40 +++++-- .../test_cloudformation_stack_integration.py | 55 ++++++++- tests/test_elbv2/test_elbv2.py | 104 +++++++++++++++++- 4 files changed, 227 insertions(+), 17 deletions(-) diff --git a/moto/elbv2/models.py b/moto/elbv2/models.py index 131862920..78897b45e 100644 --- a/moto/elbv2/models.py +++ b/moto/elbv2/models.py @@ -330,7 +330,24 @@ class FakeListenerRule(CloudFormationModel): default_actions = [] for i, action in enumerate(properties["Actions"]): action_type = action["Type"] - if action_type == "forward": + if action_type == "forward" and "ForwardConfig" in action: + action_forward_config = action["ForwardConfig"] + action_target_groups = action_forward_config["TargetGroups"] + target_group_action = [] + for action_target_group in action_target_groups: + target_group_action.append( + { + "target_group_arn": action_target_group["TargetGroupArn"], + "weight": action_target_group["Weight"], + } + ) + default_actions.append( + { + "type": action_type, + "forward_config": {"target_groups": target_group_action}, + } + ) + elif action_type == "forward" and "ForwardConfig" not in action: default_actions.append( {"type": action_type, "target_group_arn": action["TargetGroupArn"],} ) @@ -381,7 +398,19 @@ class FakeAction(BaseModel): def to_xml(self): template = Template( """{{ action.type }} - {% if action.type == "forward" %} + {% if action.type == "forward" and "forward_config" in action.data %} + + + {% for target_group in action.data["forward_config"]["target_groups"] %} + + {{ target_group["target_group_arn"] }} + {{ target_group["weight"] }} + + {% endfor %} + + + {% endif %} + {% if action.type == "forward" and "forward_config" not in action.data %} {{ action.data["target_group_arn"] }} {% elif action.type == "redirect" %} @@ -666,7 +695,7 @@ class ELBv2Backend(BaseBackend): for i, action in enumerate(actions): index = i + 1 action_type = action.type - if action_type == "forward": + if action_type == "forward" and "target_group_arn" in action.data: action_target_group_arn = action.data["target_group_arn"] if action_target_group_arn not in target_group_arns: raise ActionTargetGroupNotFoundError(action_target_group_arn) @@ -674,6 +703,16 @@ class ELBv2Backend(BaseBackend): self._validate_fixed_response_action(action, i, index) elif action_type in ["redirect", "authenticate-cognito"]: pass + # pass if listener rule has forward_config as an Action property + elif ( + action_type == "forward" + and "forward_config._target_groups.member.{}._target_group_arn".format( + index + ) + in action.data.keys() + or "forward_config" in action.data.keys() + ): + pass else: raise InvalidActionTypeError(action_type, index) diff --git a/moto/elbv2/responses.py b/moto/elbv2/responses.py index b78da0a34..e6240ae49 100644 --- a/moto/elbv2/responses.py +++ b/moto/elbv2/responses.py @@ -739,7 +739,19 @@ CREATE_RULE_TEMPLATE = """ - {% if action["type"] == "forward" %} + {% if action["type"] == "forward" and "forward_config" in action.data %} + + + {% for target_group in action.data["forward_config"]["target_groups"] %} + + {{ target_group["target_group_arn"] }} + {{ target_group["weight"] }} + + {% endfor %} + + + {% endif %} + {% if action["type"] == "forward" and "forward_config" not in action.data %} {{ action["target_group_arn"] }} {% elif action["type"] == "redirect" %} {{ action["redirect_config"] }} @@ -1235,11 +1247,10 @@ DESCRIBE_TARGET_HEALTH_TEMPLATE = """ - {% for rule in rules %} - {{ "true" if rule.is_default else "false" }} + {{ "true" if rules.is_default else "false" }} - {% for condition in rule.conditions %} + {% for condition in rules.conditions %} {{ condition["field"] }} @@ -1250,18 +1261,31 @@ SET_RULE_PRIORITIES_TEMPLATE = """ + {% if action["type"] == "forward" and "forward_config" in action.data %} + + + {% for target_group in action.data["forward_config"]["target_groups"] %} + + {{ target_group["target_group_arn"] }} + {{ target_group["weight"] }} + + {% endfor %} + + + {% endif %} + {% if action["type"] == "forward" and "forward_config" not in action.data %} {{ action["target_group_arn"] }} + {% endif %} {% endfor %} - {{ rule.arn }} + {{ rules.arn }} - {% endfor %} diff --git a/tests/test_cloudformation/test_cloudformation_stack_integration.py b/tests/test_cloudformation/test_cloudformation_stack_integration.py index d152bf6c9..8846fe445 100644 --- a/tests/test_cloudformation/test_cloudformation_stack_integration.py +++ b/tests/test_cloudformation/test_cloudformation_stack_integration.py @@ -2307,13 +2307,38 @@ def test_stack_elbv2_resources_integration(): "Type": "AWS::ElasticLoadBalancingV2::ListenerRule", "Properties": { "Actions": [ - {"Type": "forward", "TargetGroupArn": {"Ref": "mytargetgroup2"}} + { + "Type": "forward", + "ForwardConfig": { + "TargetGroups": [ + { + "TargetGroupArn": {"Ref": "mytargetgroup2"}, + "Weight": 1, + }, + { + "TargetGroupArn": {"Ref": "mytargetgroup1"}, + "Weight": 2, + }, + ] + }, + } ], "Conditions": [{"field": "path-pattern", "values": ["/*"]}], "ListenerArn": {"Ref": "listener"}, "Priority": 2, }, }, + "rule2": { + "Type": "AWS::ElasticLoadBalancingV2::ListenerRule", + "Properties": { + "Actions": [ + {"Type": "forward", "TargetGroupArn": {"Ref": "mytargetgroup2"}} + ], + "Conditions": [{"field": "host-header", "values": ["example.com"]}], + "ListenerArn": {"Ref": "listener"}, + "Priority": 30, + }, + }, "myvpc": { "Type": "AWS::EC2::VPC", "Properties": {"CidrBlock": "10.0.0.0/16"}, @@ -2395,15 +2420,39 @@ def test_stack_elbv2_resources_integration(): listener_rule = elbv2_conn.describe_rules(ListenerArn=listeners[0]["ListenerArn"])[ "Rules" ] - len(listener_rule).should.equal(2) + len(listener_rule).should.equal(3) listener_rule[0]["Priority"].should.equal("2") listener_rule[0]["Actions"].should.equal( - [{"Type": "forward", "TargetGroupArn": target_groups[1]["TargetGroupArn"]}] + [ + { + "Type": "forward", + "ForwardConfig": { + "TargetGroups": [ + { + "TargetGroupArn": target_groups[1]["TargetGroupArn"], + "Weight": 1, + }, + { + "TargetGroupArn": target_groups[0]["TargetGroupArn"], + "Weight": 2, + }, + ] + }, + } + ] ) listener_rule[0]["Conditions"].should.equal( [{"Field": "path-pattern", "Values": ["/*"]}] ) + listener_rule[1]["Priority"].should.equal("30") + listener_rule[1]["Actions"].should.equal( + [{"Type": "forward", "TargetGroupArn": target_groups[1]["TargetGroupArn"]}] + ) + listener_rule[1]["Conditions"].should.equal( + [{"Field": "host-header", "Values": ["example.com"]}] + ) + # test outputs stacks = cfn_conn.describe_stacks(StackName="elb_stack")["Stacks"] len(stacks).should.equal(1) diff --git a/tests/test_elbv2/test_elbv2.py b/tests/test_elbv2/test_elbv2.py index f3e575a34..2c3e53c19 100644 --- a/tests/test_elbv2/test_elbv2.py +++ b/tests/test_elbv2/test_elbv2.py @@ -1102,14 +1102,70 @@ def test_handle_listener_rules(): ], ) + # add rule that uses forward_config + priority = 550 + host = "aaa.example.com" + path_pattern = "barfoo" + rules = conn.create_rule( + ListenerArn=http_listener_arn, + Priority=priority, + Conditions=[ + {"Field": "host-header", "Values": [host]}, + {"Field": "path-pattern", "Values": [path_pattern]}, + { + "Field": "path-pattern", + "PathPatternConfig": {"Values": [pathpatternconfig_pattern]}, + }, + ], + Actions=[ + { + "Type": "forward", + "ForwardConfig": { + "TargetGroups": [ + { + "TargetGroupArn": target_group.get("TargetGroupArn"), + "Weight": 1, + }, + { + "TargetGroupArn": target_group.get("TargetGroupArn"), + "Weight": 2, + }, + ] + }, + }, + ], + ) + + # test for PriorityInUse + with pytest.raises(ClientError): + conn.create_rule( + ListenerArn=http_listener_arn, + Priority=priority, + Conditions=[ + {"Field": "host-header", "Values": [host]}, + {"Field": "path-pattern", "Values": [path_pattern]}, + { + "Field": "path-pattern", + "PathPatternConfig": {"Values": [pathpatternconfig_pattern]}, + }, + ], + Actions=[ + { + "TargetGroupArn": target_group.get("TargetGroupArn"), + "Type": "forward", + } + ], + ) + # test for describe listeners obtained_rules = conn.describe_rules(ListenerArn=http_listener_arn) - obtained_rules["Rules"].should.have.length_of(3) + obtained_rules["Rules"].should.have.length_of(4) priorities = [rule["Priority"] for rule in obtained_rules["Rules"]] - priorities.should.equal(["100", "500", "default"]) + priorities.should.equal(["100", "500", "550", "default"]) first_rule = obtained_rules["Rules"][0] second_rule = obtained_rules["Rules"][1] + third_rule = obtained_rules["Rules"][2] obtained_rules = conn.describe_rules(RuleArns=[first_rule["RuleArn"]]) obtained_rules["Rules"].should.equal([first_rule]) @@ -1173,11 +1229,53 @@ def test_handle_listener_rules(): } ] ) + + # modify forward_config rule partially rule + new_host_2 = "new.examplewebsite.com" + new_path_pattern_2 = "new_path_2" + new_pathpatternconfig_pattern_2 = "new_path_2" + modified_rule = conn.modify_rule( + RuleArn=third_rule["RuleArn"], + Conditions=[ + {"Field": "host-header", "Values": [new_host_2]}, + {"Field": "path-pattern", "Values": [new_path_pattern_2]}, + { + "Field": "path-pattern", + "PathPatternConfig": {"Values": [new_pathpatternconfig_pattern_2]}, + }, + ], + Actions=[ + {"TargetGroupArn": target_group.get("TargetGroupArn"), "Type": "forward",} + ], + ) + + rules = conn.describe_rules(ListenerArn=http_listener_arn) + obtained_rule = rules["Rules"][2] + obtained_rule["Conditions"][0]["Values"][0].should.equal(new_host_2) + obtained_rule["Conditions"][1]["Values"][0].should.equal(new_path_pattern_2) + obtained_rule["Conditions"][2]["Values"][0].should.equal( + new_pathpatternconfig_pattern_2 + ) + obtained_rule["Actions"][0]["TargetGroupArn"].should.equal( + target_group.get("TargetGroupArn") + ) + + # modify priority + conn.set_rule_priorities( + RulePriorities=[ + { + "RuleArn": third_rule["RuleArn"], + "Priority": int(third_rule["Priority"]) - 1, + } + ] + ) + with pytest.raises(ClientError): conn.set_rule_priorities( RulePriorities=[ {"RuleArn": first_rule["RuleArn"], "Priority": 999}, {"RuleArn": second_rule["RuleArn"], "Priority": 999}, + {"RuleArn": third_rule["RuleArn"], "Priority": 999}, ] ) @@ -1185,7 +1283,7 @@ def test_handle_listener_rules(): arn = first_rule["RuleArn"] conn.delete_rule(RuleArn=arn) rules = conn.describe_rules(ListenerArn=http_listener_arn)["Rules"] - len(rules).should.equal(2) + len(rules).should.equal(3) # test for invalid action type safe_priority = 2