[cloud] ec2_group: Handle duplicate names between EC2 classic and VPC groups (#28931)
* ec2_group: Handle name conflict with empty vpc_id. If several groups exist with the same name (and vpc_id is None) then treat the group outside the vpc as preferred (same as it would for a vpc group with vpc_id specified). Also don't run the egress rules code in that case. * Handle lack of `IpPermissionsEgress` attribute on EC2 classic groups In EC2 classic groups, the `while True` loop checking for egress permissions will continue infinitely. * Handle incompatible combinations of EC2 Classic + VPC groups * Fix integration tests in accounts lacking EC2 classic This change checks against the security group created, instead of the module parameters, for VPC ID. This means that new accounts with a default VPC will still wait properly for the first egress rule to populate. * Fix conditional for storing described groups with preference for matching VPC IDs * Revert `vpc_id is None` on conditional to allow for default VPCs
This commit is contained in:
parent
4e32c92166
commit
4bc4abfe1b
1 changed files with 27 additions and 12 deletions
|
@ -385,9 +385,17 @@ def get_target_from_rule(module, client, rule, name, group, groups, vpc_id):
|
||||||
group_id = group['GroupId']
|
group_id = group['GroupId']
|
||||||
groups[group_id] = group
|
groups[group_id] = group
|
||||||
groups[group_name] = group
|
groups[group_name] = group
|
||||||
elif group_name in groups:
|
elif group_name in groups and group.get('VpcId') and groups[group_name].get('VpcId'):
|
||||||
|
# both are VPC groups, this is ok
|
||||||
|
group_id = groups[group_name]['GroupId']
|
||||||
|
elif group_name in groups and not (group.get('VpcId') or groups[group_name].get('VpcId')):
|
||||||
|
# both are EC2 classic, this is ok
|
||||||
group_id = groups[group_name]['GroupId']
|
group_id = groups[group_name]['GroupId']
|
||||||
else:
|
else:
|
||||||
|
# if we got here, either the target group does not exist, or there
|
||||||
|
# is a mix of EC2 classic + VPC groups. Mixing of EC2 classic + VPC
|
||||||
|
# is bad, so we have to create a new SG because no compatible group
|
||||||
|
# exists
|
||||||
if not rule.get('group_desc', '').strip():
|
if not rule.get('group_desc', '').strip():
|
||||||
module.fail_json(msg="group %s will be automatically created by rule %s and "
|
module.fail_json(msg="group %s will be automatically created by rule %s and "
|
||||||
"no description was provided" % (group_name, rule))
|
"no description was provided" % (group_name, rule))
|
||||||
|
@ -633,17 +641,24 @@ def main():
|
||||||
groupName = sg['GroupName']
|
groupName = sg['GroupName']
|
||||||
if groupName in groups:
|
if groupName in groups:
|
||||||
# Prioritise groups from the current VPC
|
# Prioritise groups from the current VPC
|
||||||
if vpc_id is None or sg['VpcId'] == vpc_id:
|
# even if current VPC is EC2-Classic
|
||||||
|
if groups[groupName].get('VpcId') == vpc_id:
|
||||||
|
# Group saved already matches current VPC, change nothing
|
||||||
|
pass
|
||||||
|
elif vpc_id is None and groups[groupName].get('VpcId') is None:
|
||||||
|
# We're in EC2 classic, and the group already saved is as well
|
||||||
|
# No VPC groups can be used alongside EC2 classic groups
|
||||||
|
pass
|
||||||
|
else:
|
||||||
|
# the current SG stored has no direct match, so we can replace it
|
||||||
groups[groupName] = sg
|
groups[groupName] = sg
|
||||||
else:
|
else:
|
||||||
groups[groupName] = sg
|
groups[groupName] = sg
|
||||||
|
|
||||||
if group_id:
|
if group_id and sg['GroupId'] == group_id:
|
||||||
if sg['GroupId'] == group_id:
|
group = sg
|
||||||
group = sg
|
elif groupName == name and (vpc_id is None or sg['VpcId'] == vpc_id):
|
||||||
else:
|
group = sg
|
||||||
if groupName == name and (vpc_id is None or sg['VpcId'] == vpc_id):
|
|
||||||
group = sg
|
|
||||||
|
|
||||||
# Ensure requested group is absent
|
# Ensure requested group is absent
|
||||||
if state == 'absent':
|
if state == 'absent':
|
||||||
|
@ -685,7 +700,7 @@ def main():
|
||||||
# amazon sometimes takes a couple seconds to update the security group so wait till it exists
|
# amazon sometimes takes a couple seconds to update the security group so wait till it exists
|
||||||
while True:
|
while True:
|
||||||
group = get_security_groups_with_backoff(client, GroupIds=[group['GroupId']])['SecurityGroups'][0]
|
group = get_security_groups_with_backoff(client, GroupIds=[group['GroupId']])['SecurityGroups'][0]
|
||||||
if not group['IpPermissionsEgress']:
|
if group.get('VpcId') and not group.get('IpPermissionsEgress'):
|
||||||
pass
|
pass
|
||||||
else:
|
else:
|
||||||
break
|
break
|
||||||
|
@ -831,8 +846,8 @@ def main():
|
||||||
# If rule already exists, don't later delete it
|
# If rule already exists, don't later delete it
|
||||||
changed, ip_permission = authorize_ip("out", changed, client, group, groupRules, ipv6,
|
changed, ip_permission = authorize_ip("out", changed, client, group, groupRules, ipv6,
|
||||||
ip_permission, module, rule, "ipv6")
|
ip_permission, module, rule, "ipv6")
|
||||||
else:
|
elif vpc_id is not None:
|
||||||
# when no egress rules are specified,
|
# when no egress rules are specified and we're in a VPC,
|
||||||
# we add in a default allow all out rule, which was the
|
# we add in a default allow all out rule, which was the
|
||||||
# default behavior before egress rules were added
|
# default behavior before egress rules were added
|
||||||
default_egress_rule = 'out--1-None-None-' + group['GroupId'] + '-0.0.0.0/0'
|
default_egress_rule = 'out--1-None-None-' + group['GroupId'] + '-0.0.0.0/0'
|
||||||
|
@ -856,7 +871,7 @@ def main():
|
||||||
del groupRules[default_egress_rule]
|
del groupRules[default_egress_rule]
|
||||||
|
|
||||||
# Finally, remove anything left in the groupRules -- these will be defunct rules
|
# Finally, remove anything left in the groupRules -- these will be defunct rules
|
||||||
if purge_rules_egress:
|
if purge_rules_egress and vpc_id is not None:
|
||||||
for (rule, grant) in groupRules.values():
|
for (rule, grant) in groupRules.values():
|
||||||
# we shouldn't be revoking 0.0.0.0 egress
|
# we shouldn't be revoking 0.0.0.0 egress
|
||||||
if grant != '0.0.0.0/0':
|
if grant != '0.0.0.0/0':
|
||||||
|
|
Loading…
Reference in a new issue