diff --git a/lib/ansible/module_utils/ec2.py b/lib/ansible/module_utils/ec2.py index cc7c11d99c..31d2712d25 100644 --- a/lib/ansible/module_utils/ec2.py +++ b/lib/ansible/module_utils/ec2.py @@ -714,7 +714,7 @@ def compare_aws_tags(current_tags_dict, new_tags_dict, purge_tags=True): tag_keys_to_unset.append(key) for key in set(new_tags_dict.keys()) - set(tag_keys_to_unset): - if new_tags_dict[key] != current_tags_dict.get(key): + if to_text(new_tags_dict[key]) != current_tags_dict.get(key): tag_key_value_pairs_to_set[key] = new_tags_dict[key] return tag_key_value_pairs_to_set, tag_keys_to_unset diff --git a/lib/ansible/modules/cloud/amazon/ec2_group.py b/lib/ansible/modules/cloud/amazon/ec2_group.py index a06b3c9f38..d0ba492d5d 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_group.py +++ b/lib/ansible/modules/cloud/amazon/ec2_group.py @@ -292,6 +292,7 @@ owner_id: import json import re +import itertools from time import sleep from collections import namedtuple from ansible.module_utils.aws.core import AnsibleAWSModule, is_boto3_error_code @@ -317,7 +318,13 @@ current_account_id = None def rule_cmp(a, b): """Compare rules without descriptions""" for prop in ['port_range', 'protocol', 'target', 'target_type']: - if getattr(a, prop) != getattr(b, prop): + if prop == 'port_range' and to_text(a.protocol) == to_text(b.protocol): + # equal protocols can interchange `(-1, -1)` and `(None, None)` + if a.port_range in ((None, None), (-1, -1)) and b.port_range in ((None, None), (-1, -1)): + continue + elif getattr(a, prop) != getattr(b, prop): + return False + elif getattr(a, prop) != getattr(b, prop): return False return True @@ -390,7 +397,7 @@ def rule_from_group_permission(perm): # there may be several IP ranges here, which is ok yield Rule( ports_from_permission(perm), - perm['IpProtocol'], + to_text(perm['IpProtocol']), r[target_subkey], target_type, r.get('Description') @@ -416,7 +423,7 @@ def rule_from_group_permission(perm): yield Rule( ports_from_permission(perm), - perm['IpProtocol'], + to_text(perm['IpProtocol']), target, 'group', pair.get('Description') @@ -803,6 +810,15 @@ def wait_for_rule_propagation(module, group, desired_ingress, desired_egress, pu current_rules = set(sum([list(rule_from_group_permission(p)) for p in group[rule_key]], [])) if purge and len(current_rules ^ set(desired_rules)) == 0: return group + elif purge: + conflicts = current_rules ^ set(desired_rules) + # For cases where set comparison is equivalent, but invalid port/proto exist + for a, b in itertools.combinations(conflicts, 2): + if rule_cmp(a, b): + conflicts.discard(a) + conflicts.discard(b) + if not len(conflicts): + return group elif current_rules.issuperset(desired_rules) and not purge: return group sleep(10) @@ -1039,11 +1055,19 @@ def main(): rule['proto'] = '-1' rule['from_port'] = None rule['to_port'] = None + try: + int(rule.get('proto', 'tcp')) + rule['proto'] = to_text(rule.get('proto', 'tcp')) + rule['from_port'] = None + rule['to_port'] = None + except ValueError: + # rule does not use numeric protocol spec + pass named_tuple_rule_list.append( Rule( port_range=(rule['from_port'], rule['to_port']), - protocol=rule.get('proto', 'tcp'), + protocol=to_text(rule.get('proto', 'tcp')), target=target, target_type=target_type, description=rule.get('rule_desc'), ) diff --git a/test/integration/targets/ec2_group/tasks/main.yml b/test/integration/targets/ec2_group/tasks/main.yml index 069dc0b7aa..76b7e818c2 100644 --- a/test/integration/targets/ec2_group/tasks/main.yml +++ b/test/integration/targets/ec2_group/tasks/main.yml @@ -42,6 +42,7 @@ Name: "{{ resource_prefix }}-vpc" Description: "Created by ansible-test" register: vpc_result + - include: ./numeric_protos.yml - include: ./rule_group_create.yml - include: ./egress_tests.yml - include: ./data_validation.yml diff --git a/test/integration/targets/ec2_group/tasks/numeric_protos.yml b/test/integration/targets/ec2_group/tasks/numeric_protos.yml new file mode 100644 index 0000000000..ba4f7e90dc --- /dev/null +++ b/test/integration/targets/ec2_group/tasks/numeric_protos.yml @@ -0,0 +1,71 @@ +--- +- block: + - name: set up aws connection info + set_fact: + group_tmp_name: '{{ec2_group_name}}-numbered-protos' + 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 numbered protocol (GRE) + ec2_group: + name: '{{ group_tmp_name }}' + vpc_id: '{{ vpc_result.vpc.id }}' + description: '{{ ec2_group_description }}' + rules: + - proto: 47 + to_port: -1 + from_port: -1 + cidr_ip: 0.0.0.0/0 + <<: *aws_connection_info + state: present + register: result + + - name: Create a group with a quoted proto + ec2_group: + name: '{{ group_tmp_name }}' + vpc_id: '{{ vpc_result.vpc.id }}' + description: '{{ ec2_group_description }}' + rules: + - proto: '47' + to_port: -1 + from_port: -1 + cidr_ip: 0.0.0.0/0 + <<: *aws_connection_info + state: present + register: result + - assert: + that: + - result is not changed + - name: Add a tag with a numeric value + ec2_group: + name: '{{ group_tmp_name }}' + vpc_id: '{{ vpc_result.vpc.id }}' + description: '{{ ec2_group_description }}' + tags: + foo: 1 + <<: *aws_connection_info + - name: Read a tag with a numeric value + ec2_group: + name: '{{ group_tmp_name }}' + vpc_id: '{{ vpc_result.vpc.id }}' + description: '{{ ec2_group_description }}' + tags: + foo: 1 + <<: *aws_connection_info + register: result + - assert: + that: + - result is not changed + + always: + - name: tidy up egress rule test security group + ec2_group: + name: '{{group_tmp_name}}' + state: absent + vpc_id: '{{ vpc_result.vpc.id }}' + <<: *aws_connection_info + ignore_errors: yes