diff --git a/changelogs/fragments/59844-ec2_group-fix-diff-mode-python3.yml b/changelogs/fragments/59844-ec2_group-fix-diff-mode-python3.yml new file mode 100644 index 00000000000..541b0ba35b8 --- /dev/null +++ b/changelogs/fragments/59844-ec2_group-fix-diff-mode-python3.yml @@ -0,0 +1,2 @@ +bugfixes: + - ec2_group - Fix traceback sorting dictionaries using Python 3 and ensure rules shown by diff mode are in a consistent order. diff --git a/lib/ansible/modules/cloud/amazon/ec2_group.py b/lib/ansible/modules/cloud/amazon/ec2_group.py index 21b27d8ca9b..0669c3fdcc1 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_group.py +++ b/lib/ansible/modules/cloud/amazon/ec2_group.py @@ -956,7 +956,7 @@ def get_diff_final_resource(client, module, security_group): format_rule['user_id_group_pairs'][0].pop(k) final_rules.append(format_rule) # Order final rules consistently - final_rules.sort(key=lambda x: x.get('cidr_ip', x.get('ip_ranges', x.get('ipv6_ranges', x.get('prefix_list_ids', x.get('user_id_group_pairs')))))) + final_rules.sort(key=get_ip_permissions_sort_key) return final_rules security_group_ingress = security_group.get('ip_permissions', []) specified_ingress = module.params['rules'] @@ -996,6 +996,34 @@ def flatten_nested_targets(module, rules): return rules +def get_rule_sort_key(dicts): + if dicts.get('cidr_ip'): + return dicts.get('cidr_ip') + elif dicts.get('cidr_ipv6'): + return dicts.get('cidr_ipv6') + elif dicts.get('prefix_list_id'): + return dicts.get('prefix_list_id') + elif dicts.get('group_id'): + return dicts.get('group_id') + return None + + +def get_ip_permissions_sort_key(rule): + if rule.get('ip_ranges'): + rule.get('ip_ranges').sort(key=get_rule_sort_key) + return rule.get('ip_ranges')[0]['cidr_ip'] + elif rule.get('ipv6_ranges'): + rule.get('ipv6_ranges').sort(key=get_rule_sort_key) + return rule.get('ipv6_ranges')[0]['cidr_ipv6'] + elif rule.get('prefix_list_ids'): + rule.get('prefix_list_ids').sort(key=get_rule_sort_key) + return rule.get('prefix_list_ids')[0]['prefix_list_id'] + elif rule.get('user_id_group_pairs'): + rule.get('user_id_group_pairs').sort(key=get_rule_sort_key) + return rule.get('user_id_group_pairs')[0]['group_id'] + return None + + def main(): argument_spec = dict( name=dict(), @@ -1195,6 +1223,8 @@ def main(): if module._diff: if module.params['state'] == 'present': after = get_diff_final_resource(client, module, security_group) + if before.get('ip_permissions'): + before['ip_permissions'].sort(key=get_ip_permissions_sort_key) security_group['diff'] = [{'before': before, 'after': after}] diff --git a/test/integration/targets/ec2_group/tasks/diff_mode.yml b/test/integration/targets/ec2_group/tasks/diff_mode.yml new file mode 100644 index 00000000000..c4bf13bc14f --- /dev/null +++ b/test/integration/targets/ec2_group/tasks/diff_mode.yml @@ -0,0 +1,184 @@ +--- + - 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: create a group with a rule (CHECK MODE + DIFF) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: tcp + from_port: 80 + to_port: 80 + cidr_ip: 0.0.0.0/0 + rules_egress: + - proto: all + cidr_ip: 0.0.0.0/0 + <<: *aws_connection_info + register: check_mode_result + check_mode: true + diff: true + + - assert: + that: + - check_mode_result.changed + + - name: create a group with a rule (DIFF) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: tcp + from_port: 80 + to_port: 80 + cidr_ip: 0.0.0.0/0 + rules_egress: + - proto: all + cidr_ip: 0.0.0.0/0 + <<: *aws_connection_info + register: result + diff: true + + - assert: + that: + - result.changed + - result.diff.0.after.ip_permissions == check_mode_result.diff.0.after.ip_permissions + - result.diff.0.after.ip_permissions_egress == check_mode_result.diff.0.after.ip_permissions_egress + + - name: add rules to make sorting occur (CHECK MODE + DIFF) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: tcp + from_port: 80 + to_port: 80 + cidr_ip: 0.0.0.0/0 + - proto: tcp + from_port: 22 + to_port: 22 + cidr_ip: 20.0.0.0/8 + - proto: tcp + from_port: 22 + to_port: 22 + cidr_ip: 10.0.0.0/8 + rules_egress: + - proto: all + cidr_ip: 0.0.0.0/0 + <<: *aws_connection_info + register: check_mode_result + check_mode: true + diff: true + + - assert: + that: + - check_mode_result.changed + + - name: add rules in a different order to test sorting consistency (DIFF) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: tcp + from_port: 22 + to_port: 22 + cidr_ip: 20.0.0.0/8 + - proto: tcp + from_port: 80 + to_port: 80 + cidr_ip: 0.0.0.0/0 + - proto: tcp + from_port: 22 + to_port: 22 + cidr_ip: 10.0.0.0/8 + rules_egress: + - proto: all + cidr_ip: 0.0.0.0/0 + <<: *aws_connection_info + register: result + diff: true + + - assert: + that: + - result.changed + - result.diff.0.after.ip_permissions == check_mode_result.diff.0.after.ip_permissions + - result.diff.0.after.ip_permissions_egress == check_mode_result.diff.0.after.ip_permissions_egress + + - name: purge rules (CHECK MODE + DIFF) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: tcp + from_port: 80 + to_port: 80 + cidr_ip: 0.0.0.0/0 + rules_egress: [] + <<: *aws_connection_info + register: check_mode_result + check_mode: true + diff: true + + - assert: + that: + - check_mode_result.changed + + - name: purge rules (DIFF) + ec2_group: + name: '{{ ec2_group_name }}' + description: '{{ ec2_group_description }}' + state: present + rules: + - proto: tcp + from_port: 80 + to_port: 80 + cidr_ip: 0.0.0.0/0 + rules_egress: [] + <<: *aws_connection_info + register: result + diff: true + + - assert: + that: + - result.changed + - result.diff.0.after.ip_permissions == check_mode_result.diff.0.after.ip_permissions + - result.diff.0.after.ip_permissions_egress == check_mode_result.diff.0.after.ip_permissions_egress + + - name: delete the security group (CHECK MODE + DIFF) + ec2_group: + name: '{{ ec2_group_name }}' + state: absent + <<: *aws_connection_info + register: check_mode_result + diff: true + check_mode: true + + - assert: + that: + - check_mode_result.changed + + - name: delete the security group (DIFF) + ec2_group: + name: '{{ ec2_group_name }}' + state: absent + <<: *aws_connection_info + register: result + diff: true + + - assert: + that: + - result.changed + - not result.diff.0.after and not check_mode_result.diff.0.after diff --git a/test/integration/targets/ec2_group/tasks/main.yml b/test/integration/targets/ec2_group/tasks/main.yml index 2eb9768f1ef..e1b86683a9a 100644 --- a/test/integration/targets/ec2_group/tasks/main.yml +++ b/test/integration/targets/ec2_group/tasks/main.yml @@ -50,6 +50,7 @@ register: vpc_result #TODO(ryansb): Update CI for VPC peering permissions #- include: ./multi_account.yml + - include: ./diff_mode.yml - include: ./numeric_protos.yml - include: ./rule_group_create.yml - include: ./egress_tests.yml