From d7ca3f2bd3f6bbd0877e684fe136a02b7a74cb79 Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Mon, 17 Sep 2018 14:31:41 -0400 Subject: [PATCH] ec2_group: fix regression for targets that are a list containing strings and lists (#45594) * Fix targets that may be a list containing strings and lists which worked prior to 2.6. * Add ec2_group integration tests for lists of nested targets * changelog * Add diff mode support for lists of targets containing strings and lists. --- ...ix_target_containing_list_within_list.yaml | 6 + lib/ansible/modules/cloud/amazon/ec2_group.py | 35 ++- .../targets/ec2_group/tasks/main.yml | 1 + .../ec2_group/tasks/multi_nested_target.yml | 230 ++++++++++++++++++ 4 files changed, 268 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/ec2_group_fix_target_containing_list_within_list.yaml create mode 100644 test/integration/targets/ec2_group/tasks/multi_nested_target.yml diff --git a/changelogs/fragments/ec2_group_fix_target_containing_list_within_list.yaml b/changelogs/fragments/ec2_group_fix_target_containing_list_within_list.yaml new file mode 100644 index 0000000000..5302d5dd9f --- /dev/null +++ b/changelogs/fragments/ec2_group_fix_target_containing_list_within_list.yaml @@ -0,0 +1,6 @@ +--- +bugfixes: + - ec2_group - Sanitize the ingress and egress rules before operating on them by flattening any lists + within lists describing the target CIDR(s) into a list of strings. Prior to Ansible 2.6 the ec2_group + module accepted a list of strings, a list of lists, or a combination of strings and lists within a list. + https://github.com/ansible/ansible/pull/45594 diff --git a/lib/ansible/modules/cloud/amazon/ec2_group.py b/lib/ansible/modules/cloud/amazon/ec2_group.py index 492a71f15b..fe6984ae75 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_group.py +++ b/lib/ansible/modules/cloud/amazon/ec2_group.py @@ -293,6 +293,7 @@ owner_id: import json import re import itertools +from copy import deepcopy from time import sleep from collections import namedtuple from ansible.module_utils.aws.core import AnsibleAWSModule, is_boto3_error_code @@ -890,6 +891,7 @@ def get_diff_final_resource(client, module, security_group): final_rules = [] else: final_rules = list(security_group_rules) + specified_rules = flatten_nested_targets(module, deepcopy(specified_rules)) for rule in specified_rules: format_rule = { 'from_port': None, 'to_port': None, 'ip_protocol': rule.get('proto', 'tcp'), @@ -900,7 +902,7 @@ def get_diff_final_resource(client, module, security_group): format_rule.pop('from_port') format_rule.pop('to_port') elif rule.get('ports'): - if rule.get('ports') and isinstance(rule.get('ports'), string_types): + if rule.get('ports') and (isinstance(rule['ports'], string_types) or isinstance(rule['ports'], int)): rule['ports'] = [rule['ports']] for port in rule.get('ports'): if isinstance(port, string_types) and '-' in port: @@ -916,7 +918,9 @@ def get_diff_final_resource(client, module, security_group): if rule.get('rule_desc'): format_rule[rule_key] = [{source_type: rule[source_type], 'description': rule['rule_desc']}] else: - format_rule[rule_key] = [{source_type: rule[source_type]}] + if not isinstance(rule[source_type], list): + rule[source_type] = [rule[source_type]] + format_rule[rule_key] = [{source_type: target} for target in rule[source_type]] if rule.get('group_id') or rule.get('group_name'): rule_sg = camel_dict_to_snake_dict(group_exists(client, module, module.params['vpc_id'], rule.get('group_id'), rule.get('group_name'))[0]) format_rule['user_id_group_pairs'] = [{ @@ -952,6 +956,27 @@ def get_diff_final_resource(client, module, security_group): 'vpc_id': security_group.get('vpc_id', module.params['vpc_id'])} +def flatten_nested_targets(module, rules): + def _flatten(targets): + for target in targets: + if isinstance(target, list): + for t in _flatten(target): + yield t + elif isinstance(target, string_types): + yield target + + if rules is not None: + for rule in rules: + target_list_type = None + if isinstance(rule.get('cidr_ip'), list): + target_list_type = 'cidr_ip' + elif isinstance(rule.get('cidr_ipv6'), list): + target_list_type = 'cidr_ipv6' + if target_list_type is not None: + rule[target_list_type] = list(_flatten(rule[target_list_type])) + return rules + + def main(): argument_spec = dict( name=dict(), @@ -977,8 +1002,10 @@ def main(): group_id = module.params['group_id'] description = module.params['description'] vpc_id = module.params['vpc_id'] - rules = deduplicate_rules_args(rules_expand_sources(rules_expand_ports(module.params['rules']))) - rules_egress = deduplicate_rules_args(rules_expand_sources(rules_expand_ports(module.params['rules_egress']))) + rules = flatten_nested_targets(module, deepcopy(module.params['rules'])) + rules_egress = flatten_nested_targets(module, deepcopy(module.params['rules_egress'])) + rules = deduplicate_rules_args(rules_expand_sources(rules_expand_ports(rules))) + rules_egress = deduplicate_rules_args(rules_expand_sources(rules_expand_ports(rules_egress))) state = module.params.get('state') purge_rules = module.params['purge_rules'] purge_rules_egress = module.params['purge_rules_egress'] diff --git a/test/integration/targets/ec2_group/tasks/main.yml b/test/integration/targets/ec2_group/tasks/main.yml index e7784235c7..d34e0a4d0f 100644 --- a/test/integration/targets/ec2_group/tasks/main.yml +++ b/test/integration/targets/ec2_group/tasks/main.yml @@ -54,6 +54,7 @@ - include: ./rule_group_create.yml - include: ./egress_tests.yml - include: ./data_validation.yml + - include: ./multi_nested_target.yml # ============================================================ - name: test state=absent (CHECK MODE) diff --git a/test/integration/targets/ec2_group/tasks/multi_nested_target.yml b/test/integration/targets/ec2_group/tasks/multi_nested_target.yml new file mode 100644 index 0000000000..876f2a30a3 --- /dev/null +++ b/test/integration/targets/ec2_group/tasks/multi_nested_target.yml @@ -0,0 +1,230 @@ +--- + - name: set up aws connection info + set_fact: + aws_connection_info: &aws_connection_info + aws_access_key: "{{ aws_access_key }}" + aws_secret_key: "{{ aws_secret_key }}" + security_token: "{{ security_token }}" + region: "{{ aws_region }}" + no_log: yes + + # ============================================================ + + - name: test state=present for multiple ipv6 and ipv4 targets (expected changed=true) (CHECK MODE) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: "tcp" + from_port: 8182 + to_port: 8182 + cidr_ipv6: + - "64:ff9b::/96" + - ["2620::/32"] + - proto: "tcp" + ports: 5665 + cidr_ip: + - 172.16.1.0/24 + - 172.16.17.0/24 + - ["10.0.0.0/24", "20.0.0.0/24"] + <<: *aws_connection_info + check_mode: true + register: result + + - name: assert state=present (expected changed=true) + assert: + that: + - 'result.changed' + + - name: test state=present for multiple ipv6 and ipv4 targets (expected changed=true) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: "tcp" + from_port: 8182 + to_port: 8182 + cidr_ipv6: + - "64:ff9b::/96" + - ["2620::/32"] + - proto: "tcp" + ports: 5665 + cidr_ip: + - 172.16.1.0/24 + - 172.16.17.0/24 + - ["10.0.0.0/24", "20.0.0.0/24"] + <<: *aws_connection_info + register: result + + - name: assert state=present (expected changed=true) + assert: + that: + - 'result.changed' + - 'result.ip_permissions | length == 2' + - 'result.ip_permissions[0].ip_ranges | length == 4 or result.ip_permissions[1].ip_ranges | length == 4' + - 'result.ip_permissions[0].ipv6_ranges | length == 2 or result.ip_permissions[1].ipv6_ranges | length == 2' + + - name: test state=present for multiple ipv6 and ipv4 targets (expected changed=false) (CHECK MODE) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: "tcp" + from_port: 8182 + to_port: 8182 + cidr_ipv6: + - "64:ff9b::/96" + - ["2620::/32"] + - proto: "tcp" + ports: 5665 + cidr_ip: + - 172.16.1.0/24 + - 172.16.17.0/24 + - ["10.0.0.0/24", "20.0.0.0/24"] + <<: *aws_connection_info + check_mode: true + register: result + + - name: assert state=present (expected changed=true) + assert: + that: + - 'not result.changed' + + - name: test state=present for multiple ipv6 and ipv4 targets (expected changed=false) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: "tcp" + from_port: 8182 + to_port: 8182 + cidr_ipv6: + - "64:ff9b::/96" + - ["2620::/32"] + - proto: "tcp" + ports: 5665 + cidr_ip: + - 172.16.1.0/24 + - 172.16.17.0/24 + - ["10.0.0.0/24", "20.0.0.0/24"] + <<: *aws_connection_info + register: result + + - name: assert state=present (expected changed=true) + assert: + that: + - 'not result.changed' + + - name: test state=present purging a nested ipv4 target (expected changed=true) (CHECK MODE) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: "tcp" + from_port: 8182 + to_port: 8182 + cidr_ipv6: + - "64:ff9b::/96" + - ["2620::/32"] + - proto: "tcp" + ports: 5665 + cidr_ip: + - 172.16.1.0/24 + - 172.16.17.0/24 + - ["10.0.0.0/24"] + <<: *aws_connection_info + check_mode: true + register: result + + - assert: + that: + - result.changed + + - name: test state=present purging a nested ipv4 target (expected changed=true) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: "tcp" + from_port: 8182 + to_port: 8182 + cidr_ipv6: + - "64:ff9b::/96" + - ["2620::/32"] + - proto: "tcp" + ports: 5665 + cidr_ip: + - 172.16.1.0/24 + - 172.16.17.0/24 + - ["10.0.0.0/24"] + <<: *aws_connection_info + register: result + + - assert: + that: + - result.changed + - 'result.ip_permissions[0].ip_ranges | length == 3 or result.ip_permissions[1].ip_ranges | length == 3' + - 'result.ip_permissions[0].ipv6_ranges | length == 2 or result.ip_permissions[1].ipv6_ranges | length == 2' + + - name: test state=present with both associated ipv6 targets nested (expected changed=false) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: "tcp" + from_port: 8182 + to_port: 8182 + cidr_ipv6: + - ["2620::/32", "64:ff9b::/96"] + - proto: "tcp" + ports: 5665 + cidr_ip: + - 172.16.1.0/24 + - 172.16.17.0/24 + - ["10.0.0.0/24"] + <<: *aws_connection_info + register: result + + - assert: + that: + - not result.changed + + - name: test state=present add another nested ipv6 target (expected changed=true) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: "tcp" + from_port: 8182 + to_port: 8182 + cidr_ipv6: + - ["2620::/32", "64:ff9b::/96"] + - ["2001:DB8:A0B:12F0::1/64"] + - proto: "tcp" + ports: 5665 + cidr_ip: + - 172.16.1.0/24 + - 172.16.17.0/24 + - ["10.0.0.0/24"] + <<: *aws_connection_info + register: result + + - assert: + that: + - result.changed + - 'result.ip_permissions[0].ip_ranges | length == 3 or result.ip_permissions[1].ip_ranges | length == 3' + - 'result.ip_permissions[0].ipv6_ranges | length == 3 or result.ip_permissions[1].ipv6_ranges | length == 3' + + - name: delete it + ec2_group: + name: '{{ ec2_group_name }}' + state: absent + <<: *aws_connection_info