Fix ec2_group for numbered protocols (GRE) (#42765)

* Fix spurious `changed=True` when int is passed as tag

* Fix for all AWS module using compare_aws_tags

* Handle improperly stringified protocols and allow inconsistency between None/-1 on non-tcp protocols

* Add integration test that reproduces the same bug

* Return false if the comparsison is not equal

(cherry picked from commit 20f21779d3)
pull/45307/head
Ryan Brown 6 years ago committed by Toshio Kuratomi
parent 66759810a0
commit f0eebf9187

@ -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

@ -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'),
)

@ -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

@ -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
Loading…
Cancel
Save