Skip to content

Commit

Permalink
[aws] ec2_group multi-account and peered VPC bugfix (ansible#45296)
Browse files Browse the repository at this point in the history
* Add tests to replicate bug ansible#44788 

* Handle when userId is same account due to in-account peering

* Module defaults for main.yml

* Turn off VPC peering tests in CI
  • Loading branch information
ryansb authored Sep 6, 2018
1 parent 12e2d6d commit 079299d
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 12 deletions.
23 changes: 12 additions & 11 deletions lib/ansible/modules/cloud/amazon/ec2_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,10 +356,10 @@ def to_permission(rule):
pair = {}
if rule.target[0]:
pair['UserId'] = rule.target[0]
# groupid/groupname are mutually exclusive
if rule.target[1] and not rule.target[2]:
# group_id/group_name are mutually exclusive - give group_id more precedence as it is more specific
if rule.target[1]:
pair['GroupId'] = rule.target[1]
if rule.target[2]:
elif rule.target[2]:
pair['GroupName'] = rule.target[2]
perm['UserIdGroupPairs'] = [pair]
else:
Expand Down Expand Up @@ -405,12 +405,6 @@ def ports_from_permission(p):
if 'UserIdGroupPairs' in perm and perm['UserIdGroupPairs']:
for pair in perm['UserIdGroupPairs']:
target = pair['GroupId']
if pair.get('UserId') and pair['UserId'] != current_account_id:
target = (
pair.get('UserId', None),
pair.get('GroupId', None),
pair.get('GroupName', None),
)
if pair.get('UserId', '').startswith('amazon-'):
# amazon-elb and amazon-prefix rules don't need
# group-id specified, so remove it when querying
Expand All @@ -420,6 +414,12 @@ def ports_from_permission(p):
None,
target[2],
)
elif 'VpcPeeringConnectionId' in pair or pair['UserId'] != current_account_id:
target = (
pair.get('UserId', None),
pair.get('GroupId', None),
pair.get('GroupName', None),
)

yield Rule(
ports_from_permission(perm),
Expand Down Expand Up @@ -492,14 +492,15 @@ def get_target_from_rule(module, client, rule, name, group, groups, vpc_id):
target_group_created = False

validate_rule(module, rule)
if rule.get('group_id') and re.match(FOREIGN_SECURITY_GROUP_REGEX, rule['group_id']) and current_account_id not in rule['group_id']:
if rule.get('group_id') and re.match(FOREIGN_SECURITY_GROUP_REGEX, rule['group_id']):
# this is a foreign Security Group. Since you can't fetch it you must create an instance of it
owner_id, group_id, group_name = re.match(FOREIGN_SECURITY_GROUP_REGEX, rule['group_id']).groups()
group_instance = dict(UserId=owner_id, GroupId=group_id, GroupName=group_name)
groups[group_id] = group_instance
groups[group_name] = group_instance
# group_id/group_name are mutually exclusive - give group_id more precedence as it is more specific
if group_id and group_name:
group_id = None
group_name = None
return 'group', (owner_id, group_id, group_name), False
elif 'group_id' in rule:
return 'group', rule['group_id'], False
Expand Down
10 changes: 9 additions & 1 deletion test/integration/targets/ec2_group/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@
# - include: ../../setup_ec2/tasks/common.yml module_name: ec2_group

- include: ./credential_tests.yml
- block:
- module_defaults:
group/aws:
aws_access_key: "{{ aws_access_key }}"
aws_secret_key: "{{ aws_secret_key }}"
security_token: "{{ security_token }}"
region: "{{ aws_region }}"
block:
# ============================================================
- name: set up aws connection info
set_fact:
Expand Down Expand Up @@ -42,6 +48,8 @@
Name: "{{ resource_prefix }}-vpc"
Description: "Created by ansible-test"
register: vpc_result
#TODO(ryansb): Update CI for VPC peering permissions
#- include: ./multi_account.yml
- include: ./numeric_protos.yml
- include: ./rule_group_create.yml
- include: ./egress_tests.yml
Expand Down
124 changes: 124 additions & 0 deletions test/integration/targets/ec2_group/tasks/multi_account.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
- block:
- aws_caller_facts:
register: caller_facts
- name: create a VPC
ec2_vpc_net:
name: "{{ resource_prefix }}-vpc-2"
state: present
cidr_block: "10.232.233.128/26"
tags:
Description: "Created by ansible-test"
register: vpc_result_2
- name: Peer the secondary-VPC to the main VPC
ec2_vpc_peer:
vpc_id: '{{ vpc_result_2.vpc.id }}'
peer_vpc_id: '{{ vpc_result.vpc.id }}'
peer_owner_id: '{{ caller_facts.account }}'
peer_region: '{{ aws_region }}'
register: peer_origin
- name: Accept the secondary-VPC peering connection in the main VPC
ec2_vpc_peer:
peer_vpc_id: '{{ vpc_result_2.vpc.id }}'
vpc_id: '{{ vpc_result.vpc.id }}'
state: accept
peering_id: '{{ peer_origin.peering_id }}'
peer_owner_id: '{{ caller_facts.account }}'
peer_region: '{{ aws_region }}'
- name: Create group in second VPC
ec2_group:
name: '{{ ec2_group_name }}-external'
description: '{{ ec2_group_description }}'
vpc_id: '{{ vpc_result_2.vpc.id }}'
state: present
rules:
- proto: "tcp"
cidr_ip: 0.0.0.0/0
ports:
- 80
rule_desc: 'http whoo'
register: external
- name: Create group in internal VPC
ec2_group:
name: '{{ ec2_group_name }}-internal'
description: '{{ ec2_group_description }}'
vpc_id: '{{ vpc_result.vpc.id }}'
state: present
rules:
- proto: "tcp"
group_id: '{{ caller_facts.account }}/{{ external.group_id }}/{{ ec2_group_name }}-external'
ports:
- 80
- name: Re-make same rule, expecting changed=false in internal VPC
ec2_group:
name: '{{ ec2_group_name }}-internal'
description: '{{ ec2_group_description }}'
vpc_id: '{{ vpc_result.vpc.id }}'
state: present
rules:
- proto: "tcp"
group_id: '{{ caller_facts.account }}/{{ external.group_id }}/{{ ec2_group_name }}-external'
ports:
- 80
register: out
- assert:
that:
- out is not changed
- name: Try again with a bad group_id group in internal VPC
ec2_group:
name: '{{ ec2_group_name }}-internal'
description: '{{ ec2_group_description }}'
vpc_id: '{{ vpc_result.vpc.id }}'
state: present
rules:
- proto: "tcp"
group_id: '{{ external.group_id }}/{{ caller_facts.account }}/{{ ec2_group_name }}-external'
ports:
- 80
register: out
ignore_errors: true
- assert:
that:
- out is failed
always:
- pause: seconds=5
- name: Delete secondary-VPC side of peer
ec2_vpc_peer:
vpc_id: '{{ vpc_result_2.vpc.id }}'
peer_vpc_id: '{{ vpc_result.vpc.id }}'
peering_id: '{{ peer_origin.peering_id }}'
state: absent
peer_owner_id: '{{ caller_facts.account }}'
peer_region: '{{ aws_region }}'
ignore_errors: yes
- name: Delete main-VPC side of peer
ec2_vpc_peer:
peer_vpc_id: '{{ vpc_result_2.vpc.id }}'
vpc_id: '{{ vpc_result.vpc.id }}'
state: absent
peering_id: '{{ peer_origin.peering_id }}'
peer_owner_id: '{{ caller_facts.account }}'
peer_region: '{{ aws_region }}'
ignore_errors: yes
- name: Clean up group in second VPC
ec2_group:
name: '{{ ec2_group_name }}-external'
description: '{{ ec2_group_description }}'
state: absent
vpc_id: '{{ vpc_result_2.vpc.id }}'
ignore_errors: yes
- name: Clean up group in second VPC
ec2_group:
name: '{{ ec2_group_name }}-internal'
description: '{{ ec2_group_description }}'
state: absent
vpc_id: '{{ vpc_result.vpc.id }}'
ignore_errors: yes
- name: tidy up VPC
ec2_vpc_net:
name: "{{ resource_prefix }}-vpc-2"
state: absent
cidr_block: "10.232.233.128/26"
ignore_errors: yes
register: removed
retries: 10
until: removed is not failed

0 comments on commit 079299d

Please sign in to comment.