From 282e743eb00292588e4945d9a75307e6629dc974 Mon Sep 17 00:00:00 2001 From: Michael Tinning Date: Wed, 23 Aug 2017 18:49:52 +0100 Subject: [PATCH] elb_application_lb: fix issue with boto parameter validation on Rules (#27333) --- .../cloud/amazon/elb_application_lb.py | 14 +- .../cloud/amazon/test_elb_application_lb.py | 153 ++++++++++++++++++ 2 files changed, 163 insertions(+), 4 deletions(-) create mode 100644 test/units/modules/cloud/amazon/test_elb_application_lb.py diff --git a/lib/ansible/modules/cloud/amazon/elb_application_lb.py b/lib/ansible/modules/cloud/amazon/elb_application_lb.py index 4a85e2d559..eb88056058 100644 --- a/lib/ansible/modules/cloud/amazon/elb_application_lb.py +++ b/lib/ansible/modules/cloud/amazon/elb_application_lb.py @@ -685,8 +685,8 @@ def compare_rules(connection, module, current_listeners, listener): if modified_rule: modified_rule['Priority'] = int(current_rule['Priority']) modified_rule['RuleArn'] = current_rule['RuleArn'] - modified_rule['Actions'] = current_rule['Actions'] - modified_rule['Conditions'] = current_rule['Conditions'] + modified_rule['Actions'] = new_rule['Actions'] + modified_rule['Conditions'] = new_rule['Conditions'] rules_to_modify.append(modified_rule) break @@ -716,7 +716,11 @@ def create_or_update_elb_listeners(connection, module, elb): for listener_to_add in listeners_to_add: try: listener_to_add['LoadBalancerArn'] = elb['LoadBalancerArn'] - connection.create_listener(**listener_to_add) + # Rules is not a valid parameter for create_listener + listener_to_add.pop('Rules') + response = connection.create_listener(**listener_to_add) + # Add the new listener + current_listeners.append(response['Listeners'][0]) listener_changed = True except ClientError as e: module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response)) @@ -724,6 +728,8 @@ def create_or_update_elb_listeners(connection, module, elb): # Modify listeners for listener_to_modify in listeners_to_modify: try: + # Rules is not a valid parameter for modify_listener + listener_to_modify.pop('Rules') connection.modify_listener(**listener_to_modify) listener_changed = True except ClientError as e: @@ -738,7 +744,7 @@ def create_or_update_elb_listeners(connection, module, elb): module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response)) # For each listener, check rules - for listener in listeners: + for listener in deepcopy(listeners): if 'Rules' in listener: # Ensure rules are using Target Group ARN not name listener['Rules'] = ensure_rules_action_has_arn(connection, module, listener['Rules']) diff --git a/test/units/modules/cloud/amazon/test_elb_application_lb.py b/test/units/modules/cloud/amazon/test_elb_application_lb.py new file mode 100644 index 0000000000..ed9c738801 --- /dev/null +++ b/test/units/modules/cloud/amazon/test_elb_application_lb.py @@ -0,0 +1,153 @@ +# +# (c) 2017 Michael Tinning +# +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import (absolute_import, division, print_function) + +from nose.plugins.skip import SkipTest +import json +import pytest +from copy import deepcopy +from ansible.module_utils._text import to_bytes +from ansible.module_utils import basic +from ansible.module_utils.ec2 import HAS_BOTO3 + +if not HAS_BOTO3: + raise SkipTest("test_elb_application_lb.py requires the `boto3` and `botocore` modules") + +import ansible.modules.cloud.amazon.elb_application_lb as elb_module + + +@pytest.fixture +def listener(): + return { + 'Protocol': 'HTTP', + 'Port': 80, + 'DefaultActions': [{ + 'Type': 'forward', + 'TargetGroupName': 'target-group' + }], + 'Rules': [{ + 'Conditions': [{ + 'Field': 'host-header', + 'Values': [ + 'www.example.com' + ] + }], + 'Priority': 1, + 'Actions': [{ + 'TargetGroupName': 'other-target-group', + 'Type': 'forward' + }] + }] + } + + +@pytest.fixture +def compare_listeners(mocker): + return mocker.Mock() + + +@pytest.fixture +def ensure_listeners(mocker): + ensure_listeners_mock = mocker.Mock() + ensure_listeners_mock.return_value = [] + return ensure_listeners_mock + + +@pytest.fixture +def compare_rules(mocker): + compare_rules_mock = mocker.Mock() + compare_rules_mock.return_value = ([], [], []) + return compare_rules_mock + + +@pytest.fixture +def get_elb_listeners(mocker): + get_elb_listeners_mock = mocker.Mock() + get_elb_listeners_mock.return_value = [] + return get_elb_listeners_mock + + +@pytest.fixture +def elb(mocker, monkeypatch, compare_listeners, ensure_listeners, compare_rules, get_elb_listeners): + monkeypatch.setattr(elb_module, "ensure_listeners_default_action_has_arn", ensure_listeners) + monkeypatch.setattr(elb_module, "get_elb_listeners", get_elb_listeners) + monkeypatch.setattr(elb_module, "ensure_rules_action_has_arn", mocker.Mock()) + monkeypatch.setattr(elb_module, "get_listener", mocker.Mock()) + monkeypatch.setattr(elb_module, "compare_rules", compare_rules) + monkeypatch.setattr(elb_module, "compare_listeners", compare_listeners) + return elb_module + + +@pytest.fixture +def created_listener(mocker, listener): + return { + 'Port': listener['Port'], + 'ListenerArn': 'new-listener-arn' + } + + +@pytest.fixture +def connection(mocker, created_listener): + connection_mock = mocker.Mock() + connection_mock.create_listener.return_value = { + 'Listeners': [created_listener] + } + return connection_mock + + +@pytest.fixture +def existing_elb(): + return {'LoadBalancerArn': 'fake'} + + +def test_create_listeners_called_with_correct_args(mocker, connection, listener, elb, compare_listeners, existing_elb): + compare_listeners.return_value = ([listener], [], []) + + elb.create_or_update_elb_listeners(connection, mocker.Mock(), existing_elb) + + connection.create_listener.assert_called_once_with( + Protocol=listener['Protocol'], + Port=listener['Port'], + DefaultActions=listener['DefaultActions'], + LoadBalancerArn=existing_elb['LoadBalancerArn'] + ) + + +def test_modify_listeners_called_with_correct_args(mocker, connection, listener, elb, compare_listeners, existing_elb): + # In the case of modify listener, LoadBalancerArn is set in compare_listeners + listener['LoadBalancerArn'] = existing_elb['LoadBalancerArn'] + compare_listeners.return_value = ([], [listener], []) + + elb.create_or_update_elb_listeners(connection, mocker.Mock(), existing_elb) + + connection.modify_listener.assert_called_once_with( + Protocol=listener['Protocol'], + Port=listener['Port'], + DefaultActions=listener['DefaultActions'], + LoadBalancerArn=existing_elb['LoadBalancerArn'] + ) + + +def test_compare_rules_called_with_new_listener( + mocker, + connection, + listener, + elb, + compare_listeners, + ensure_listeners, + compare_rules, + existing_elb, + created_listener +): + compare_listeners.return_value = ([listener], [], []) + listener_from_ensure_listeners = deepcopy(listener) + ensure_listeners.return_value = [listener_from_ensure_listeners] + + elb.create_or_update_elb_listeners(connection, mocker.Mock(), existing_elb) + + (_conn, _module, current_listeners, _listener), _kwargs = compare_rules.call_args + + assert created_listener in current_listeners