mirror of
https://github.com/ansible-collections/community.general.git
synced 2025-07-25 14:20:22 -07:00
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
This commit is contained in:
parent
038fd0d0f2
commit
20f21779d3
4 changed files with 101 additions and 5 deletions
|
@ -714,7 +714,7 @@ def compare_aws_tags(current_tags_dict, new_tags_dict, purge_tags=True):
|
||||||
tag_keys_to_unset.append(key)
|
tag_keys_to_unset.append(key)
|
||||||
|
|
||||||
for key in set(new_tags_dict.keys()) - set(tag_keys_to_unset):
|
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]
|
tag_key_value_pairs_to_set[key] = new_tags_dict[key]
|
||||||
|
|
||||||
return tag_key_value_pairs_to_set, tag_keys_to_unset
|
return tag_key_value_pairs_to_set, tag_keys_to_unset
|
||||||
|
|
|
@ -292,6 +292,7 @@ owner_id:
|
||||||
|
|
||||||
import json
|
import json
|
||||||
import re
|
import re
|
||||||
|
import itertools
|
||||||
from time import sleep
|
from time import sleep
|
||||||
from collections import namedtuple
|
from collections import namedtuple
|
||||||
from ansible.module_utils.aws.core import AnsibleAWSModule, is_boto3_error_code
|
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):
|
def rule_cmp(a, b):
|
||||||
"""Compare rules without descriptions"""
|
"""Compare rules without descriptions"""
|
||||||
for prop in ['port_range', 'protocol', 'target', 'target_type']:
|
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 False
|
||||||
return True
|
return True
|
||||||
|
|
||||||
|
@ -390,7 +397,7 @@ def rule_from_group_permission(perm):
|
||||||
# there may be several IP ranges here, which is ok
|
# there may be several IP ranges here, which is ok
|
||||||
yield Rule(
|
yield Rule(
|
||||||
ports_from_permission(perm),
|
ports_from_permission(perm),
|
||||||
perm['IpProtocol'],
|
to_text(perm['IpProtocol']),
|
||||||
r[target_subkey],
|
r[target_subkey],
|
||||||
target_type,
|
target_type,
|
||||||
r.get('Description')
|
r.get('Description')
|
||||||
|
@ -416,7 +423,7 @@ def rule_from_group_permission(perm):
|
||||||
|
|
||||||
yield Rule(
|
yield Rule(
|
||||||
ports_from_permission(perm),
|
ports_from_permission(perm),
|
||||||
perm['IpProtocol'],
|
to_text(perm['IpProtocol']),
|
||||||
target,
|
target,
|
||||||
'group',
|
'group',
|
||||||
pair.get('Description')
|
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]], []))
|
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:
|
if purge and len(current_rules ^ set(desired_rules)) == 0:
|
||||||
return group
|
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:
|
elif current_rules.issuperset(desired_rules) and not purge:
|
||||||
return group
|
return group
|
||||||
sleep(10)
|
sleep(10)
|
||||||
|
@ -1039,11 +1055,19 @@ def main():
|
||||||
rule['proto'] = '-1'
|
rule['proto'] = '-1'
|
||||||
rule['from_port'] = None
|
rule['from_port'] = None
|
||||||
rule['to_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(
|
named_tuple_rule_list.append(
|
||||||
Rule(
|
Rule(
|
||||||
port_range=(rule['from_port'], rule['to_port']),
|
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,
|
target=target, target_type=target_type,
|
||||||
description=rule.get('rule_desc'),
|
description=rule.get('rule_desc'),
|
||||||
)
|
)
|
||||||
|
|
|
@ -42,6 +42,7 @@
|
||||||
Name: "{{ resource_prefix }}-vpc"
|
Name: "{{ resource_prefix }}-vpc"
|
||||||
Description: "Created by ansible-test"
|
Description: "Created by ansible-test"
|
||||||
register: vpc_result
|
register: vpc_result
|
||||||
|
- include: ./numeric_protos.yml
|
||||||
- include: ./rule_group_create.yml
|
- include: ./rule_group_create.yml
|
||||||
- include: ./egress_tests.yml
|
- include: ./egress_tests.yml
|
||||||
- include: ./data_validation.yml
|
- include: ./data_validation.yml
|
||||||
|
|
71
test/integration/targets/ec2_group/tasks/numeric_protos.yml
Normal file
71
test/integration/targets/ec2_group/tasks/numeric_protos.yml
Normal file
|
@ -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…
Add table
Add a link
Reference in a new issue