From 079299db4d5bec87dc873acb338c25493a0010ef Mon Sep 17 00:00:00 2001 From: Ryan Brown Date: Thu, 6 Sep 2018 15:06:03 -0400 Subject: [PATCH] [aws] ec2_group multi-account and peered VPC bugfix (#45296) * Add tests to replicate bug #44788 * Handle when userId is same account due to in-account peering * Module defaults for main.yml * Turn off VPC peering tests in CI --- lib/ansible/modules/cloud/amazon/ec2_group.py | 23 ++-- .../targets/ec2_group/tasks/main.yml | 10 +- .../targets/ec2_group/tasks/multi_account.yml | 124 ++++++++++++++++++ 3 files changed, 145 insertions(+), 12 deletions(-) create mode 100644 test/integration/targets/ec2_group/tasks/multi_account.yml diff --git a/lib/ansible/modules/cloud/amazon/ec2_group.py b/lib/ansible/modules/cloud/amazon/ec2_group.py index d0ba492d5de..492a71f15b1 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_group.py +++ b/lib/ansible/modules/cloud/amazon/ec2_group.py @@ -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: @@ -405,12 +405,6 @@ def rule_from_group_permission(perm): 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 @@ -420,6 +414,12 @@ def rule_from_group_permission(perm): 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), @@ -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 diff --git a/test/integration/targets/ec2_group/tasks/main.yml b/test/integration/targets/ec2_group/tasks/main.yml index 76b7e818c2f..e7784235c7c 100644 --- a/test/integration/targets/ec2_group/tasks/main.yml +++ b/test/integration/targets/ec2_group/tasks/main.yml @@ -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: @@ -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 diff --git a/test/integration/targets/ec2_group/tasks/multi_account.yml b/test/integration/targets/ec2_group/tasks/multi_account.yml new file mode 100644 index 00000000000..273e13cfb99 --- /dev/null +++ b/test/integration/targets/ec2_group/tasks/multi_account.yml @@ -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