[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

@@ -356,10 +356,10 @@ def to_permission(rule):
             pair = {}
             if rule.target[0]:
                 pair['UserId'] = rule.target[0]
-            # groupid/groupname are mutually exclusive
+            # group_id/group_name are mutually exclusive - give group_id more precedence as it is more specific
-            if rule.target[1] and not rule.target[2]:
+            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

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

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