[cloud] fix VPC behavior for ec2_group module, improve integration tests (#27038)

* Add tests for group in a VPC * Improve ec2_group output and documentation Update ec2_group to provide full security group information Add RETURN documentation to match * Fix ec2_group creation within a VPC Ensure VPC ID gets passed when creating security group * Add test for auto creating SG * Fix ec2_group auto group creation * Add backoff to describe_security_groups Getting LimitExceeded from describe_security_groups is definitely possible (source: me) so add backoff to increase likelihood of success. To ensure that all `describe_security_group` calls are backed off, remove implicit ones that use `ec2.SecurityGroup`. From there, the decision to remove the `ec2` boto3 resource and rely on the client alone makes good sense. * Tidy up auto created security group Add resource_prefix to auto created security group and delete it in the `always` section. Use YAML argument form for all module parameters
7 years ago · f972994662
parent 2d734c7ea7
commit f972994662
3 changed files with 280 additions and 114 deletions
--- a/hacking/aws_config/testing_policies/ec2-policy.json
+++ b/hacking/aws_config/testing_policies/ec2-policy.json
@ -20,6 +20,7 @@
                "ec2:CreateVpc",
                "ec2:DeleteKeyPair",
                "ec2:DeleteNatGateway",
+                "ec2:DeleteVpc",
                "ec2:Describe*",
                "ec2:DisassociateAddress",
                "ec2:DisassociateRouteTable",
--- a/lib/ansible/modules/cloud/amazon/ec2_group.py
+++ b/lib/ansible/modules/cloud/amazon/ec2_group.py
@ -191,6 +191,65 @@ EXAMPLES = '''
    state: absent
 '''

+RETURN = '''
+group_name:
+  description: Security group name
+  sample: My Security Group
+  type: string
+  returned: on create/update
+group_id:
+  description: Security group id
+  sample: sg-abcd1234
+  type: string
+  returned: on create/update
+description:
+  description: Description of security group
+  sample: My Security Group
+  type: string
+  returned: on create/update
+tags:
+  description: Tags associated with the security group
+  sample:
+    Name: My Security Group
+    Purpose: protecting stuff
+  type: dict
+  returned: on create/update
+vpc_id:
+  description: ID of VPC to which the security group belongs
+  sample: vpc-abcd1234
+  type: string
+  returned: on create/update
+ip_permissions:
+  description: Inbound rules associated with the security group.
+  sample:
+    - from_port: 8182
+      ip_protocol: tcp
+      ip_ranges:
+        - cidr_ip: "1.1.1.1/32"
+      ipv6_ranges: []
+      prefix_list_ids: []
+      to_port: 8182
+      user_id_group_pairs: []
+  type: list
+  returned: on create/update
+ip_permissions_egress:
+  description: Outbound rules associated with the security group.
+  sample:
+    - ip_protocol: -1
+      ip_ranges:
+        - cidr_ip: "0.0.0.0/0"
+          ipv6_ranges: []
+          prefix_list_ids: []
+          user_id_group_pairs: []
+  type: list
+  returned: on create/update
+owner_id:
+  description: AWS Account ID of the security group
+  sample: 123456789012
+  type: int
+  returned: on create/update
+'''
+
 import json
 import re
 import time
@ -200,6 +259,8 @@ from ansible.module_utils.ec2 import get_aws_connection_info
 from ansible.module_utils.ec2 import ec2_argument_spec
 from ansible.module_utils.ec2 import camel_dict_to_snake_dict
 from ansible.module_utils.ec2 import HAS_BOTO3
+from ansible.module_utils.ec2 import boto3_tag_list_to_ansible_dict
+from ansible.module_utils.ec2 import AWSRetry
 import traceback

 try:
@ -208,6 +269,11 @@ except ImportError:
    pass  # caught by imported HAS_BOTO3


+@AWSRetry.backoff(tries=5, delay=5, backoff=2.0)
+def get_security_groups_with_backoff(connection, **kwargs):
+    return connection.describe_security_groups(**kwargs)
+
+
 def deduplicate_rules_args(rules):
    """Returns unique rules"""
    if rules is None:
@ -227,7 +293,7 @@ def make_rule_key(prefix, rule, group_id, cidr_ip):
    return key.lower().replace('-none', '-None')


-def add_rules_to_loopkup(ipPermissions, group_id, prefix, dict):
+def add_rules_to_lookup(ipPermissions, group_id, prefix, dict):
    for rule in ipPermissions:
        for groupGrant in rule.get('UserIdGroupPairs'):
            dict[make_rule_key(prefix, rule, group_id, groupGrant.get('GroupId'))] = (rule, groupGrant)
@ -261,7 +327,7 @@ def validate_rule(module, rule):
        module.fail_json(msg='Specify group_id OR group_name, not both')


-def get_target_from_rule(module, ec2, rule, name, group, groups, vpc_id):
+def get_target_from_rule(module, client, rule, name, group, groups, vpc_id):
    """
    Returns tuple of (group_id, ip) after validating rule params.

@ -293,10 +359,10 @@ def get_target_from_rule(module, ec2, rule, name, group, groups, vpc_id):
        module.fail_json(msg="Specify group_id OR group_name, not both")
    elif 'cidr_ip' in rule and 'cidr_ipv6' in rule:
        module.fail_json(msg="Specify cidr_ip OR cidr_ipv6, not both")
-    elif 'group_id' in rule and re.match(FOREIGN_SECURITY_GROUP_REGEX, rule['group_id']):
+    elif 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 = ec2.SecurityGroup(owner_id=owner_id, group_name=group_name, id=group_id)
+        group_instance = dict(GroupId=group_id, GroupName=group_name)
        groups[group_id] = group_instance
        groups[group_name] = group_instance
    elif 'group_id' in rule:
@ -304,18 +370,21 @@ def get_target_from_rule(module, ec2, rule, name, group, groups, vpc_id):
    elif 'group_name' in rule:
        group_name = rule['group_name']
        if group_name == name:
-            group_id = group.id
+            group_id = group['GroupId']
            groups[group_id] = group
            groups[group_name] = group
-        elif group_name in groups and (vpc_id is None or groups[group_name].vpc_id == vpc_id):
-            group_id = groups[group_name].id
+        elif group_name in groups and (vpc_id is None or groups[group_name]['VpcId'] == vpc_id):
+            group_id = groups[group_name]['GroupId']
        else:
            if not rule.get('group_desc', '').strip():
                module.fail_json(msg="group %s will be automatically created by rule %s and "
                                     "no description was provided" % (group_name, rule))
            if not module.check_mode:
-                auto_group = ec2.create_security_group(group_name, rule['group_desc'], vpc_id=vpc_id)
-                group_id = auto_group.id
+                params = dict(GroupName=group_name, Description=rule['group_desc'])
+                if vpc_id:
+                    params['VpcId'] = vpc_id
+                auto_group = client.create_security_group(**params)
+                group_id = auto_group['GroupId']
                groups[group_id] = auto_group
                groups[group_name] = auto_group
            target_group_created = True
@ -404,7 +473,7 @@ def authorize_ip(type, changed, client, group, groupRules,
                 ip, ip_permission, module, rule, ethertype):
    # If rule already exists, don't later delete it
    for thisip in ip:
-        rule_id = make_rule_key(type, rule, group.id, thisip)
+        rule_id = make_rule_key(type, rule, group['GroupId'], thisip)
        if rule_id in groupRules:
            del groupRules[rule_id]
        else:
@ -413,14 +482,14 @@ def authorize_ip(type, changed, client, group, groupRules,
                if ip_permission:
                    try:
                        if type == "in":
-                            client.authorize_security_group_ingress(GroupId=group.group_id,
+                            client.authorize_security_group_ingress(GroupId=group['GroupId'],
                                                                    IpPermissions=[ip_permission])
                        elif type == "out":
-                            client.authorize_security_group_egress(GroupId=group.group_id,
+                            client.authorize_security_group_egress(GroupId=group['GroupId'],
                                                                   IpPermissions=[ip_permission])
                    except botocore.exceptions.ClientError as e:
                        module.fail_json(msg="Unable to authorize %s for ip %s security group '%s' - %s" %
-                                             (type, thisip, group.group_name, e),
+                                             (type, thisip, group['GroupName'], e),
                                         exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
            changed = True
    return changed, ip_permission
@ -516,16 +585,15 @@ def main():
        module.fail_json(msg="The AWS region must be specified as an "
                             "environment variable or in the AWS credentials "
                             "profile.")
-    client, ec2 = boto3_conn(module, conn_type='both', resource='ec2', endpoint=ec2_url, region=region, **aws_connect_params)
+    client = boto3_conn(module, conn_type='client', resource='ec2', endpoint=ec2_url, region=region, **aws_connect_params)
    group = None
    groups = dict()
    security_groups = []
    # do get all security groups
    # find if the group is present
    try:
-        response = client.describe_security_groups()
-        if 'SecurityGroups' in response:
-            security_groups = response.get('SecurityGroups')
+        response = get_security_groups_with_backoff(client)
+        security_groups = response.get('SecurityGroups', [])
    except botocore.exceptions.NoCredentialsError as e:
        module.fail_json(msg="Error in describe_security_groups: %s" % "Unable to locate credentials", exception=traceback.format_exc())
    except botocore.exceptions.ClientError as e:
@ -533,22 +601,21 @@ def main():
                         **camel_dict_to_snake_dict(e.response))

    for sg in security_groups:
-        curGroup = ec2.SecurityGroup(sg['GroupId'])
-        groups[curGroup.id] = ec2.SecurityGroup(curGroup.id)
-        groupName = curGroup.group_name
+        groups[sg['GroupId']] = sg
+        groupName = sg['GroupName']
        if groupName in groups:
            # Prioritise groups from the current VPC
-            if vpc_id is None or curGroup.vpc_id == vpc_id:
-                groups[groupName] = curGroup
+            if vpc_id is None or sg['VpcId'] == vpc_id:
+                groups[groupName] = sg
        else:
-            groups[groupName] = curGroup
+            groups[groupName] = sg

        if group_id:
-            if curGroup.id == group_id:
-                group = curGroup
+            if sg['GroupId'] == group_id:
+                group = sg
        else:
-            if groupName == name and (vpc_id is None or curGroup.vpc_id == vpc_id):
-                group = curGroup
+            if groupName == name and (vpc_id is None or sg['VpcId'] == vpc_id):
+                group = sg

    # Ensure requested group is absent
    if state == 'absent':
@ -556,7 +623,7 @@ def main():
            # found a match, delete it
            try:
                if not module.check_mode:
-                    group.delete()
+                    client.delete_security_group(GroupId=group['GroupId'])
            except botocore.exceptions.ClientError as e:
                module.fail_json(msg="Unable to delete security group '%s' - %s" % (group, e),
                                 exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
@ -571,7 +638,7 @@ def main():
    elif state == 'present':
        if group:
            # existing group
-            if group.description != description:
+            if group['Description'] != description:
                module.fail_json(
                    msg="Group description does not match existing group. ec2_group does not support this case.")

@ -579,18 +646,22 @@ def main():
        else:
            # no match found, create it
            if not module.check_mode:
-                group = client.create_security_group(GroupName=name, Description=description)
-                groupId = group.get('GroupId')
+                params = dict(GroupName=name, Description=description)
+                if vpc_id:
+                    params['VpcId'] = vpc_id
+                group = client.create_security_group(**params)
                # When a group is created, an egress_rule ALLOW ALL
                # to 0.0.0.0/0 is added automatically but it's not
                # reflected in the object returned by the AWS API
                # call. We re-read the group for getting an updated object
                # amazon sometimes takes a couple seconds to update the security group so wait till it exists
-                while len(client.describe_security_groups(GroupIds=[groupId])
-                          ['SecurityGroups'][0]['IpPermissionsEgress']) == 0:
-                    time.sleep(0.1)
+                while True:
+                    group = get_security_groups_with_backoff(client, GroupIds=[group['GroupId']])['SecurityGroups'][0]
+                    if not group['IpPermissionsEgress']:
+                        time.sleep(0.1)
+                    else:
+                        break

-                group = ec2.SecurityGroup(groupId)
            changed = True
    else:
        module.fail_json(msg="Unsupported state requested: %s" % state)
@ -599,13 +670,13 @@ def main():
    if group:
        # Manage ingress rules
        groupRules = {}
-        add_rules_to_loopkup(group.ip_permissions, group.id, 'in', groupRules)
+        add_rules_to_lookup(group['IpPermissions'], group['GroupId'], 'in', groupRules)
        # Now, go through all provided rules and ensure they are there.
        if rules is not None:
            ip_permission = []
            for rule in rules:
                validate_rule(module, rule)
-                group_id, ip, ipv6, target_group_created = get_target_from_rule(module, ec2, rule, name,
+                group_id, ip, ipv6, target_group_created = get_target_from_rule(module, client, rule, name,
                                                                                group, groups, vpc_id)
                if target_group_created:
                    changed = True
@ -616,7 +687,7 @@ def main():
                    rule['to_port'] = None

                if group_id:
-                    rule_id = make_rule_key('in', rule, group.id, group_id)
+                    rule_id = make_rule_key('in', rule, group['GroupId'], group_id)
                    if rule_id in groupRules:
                        del groupRules[rule_id]
                    else:
@ -628,11 +699,11 @@ def main():
                                    [useridpair.update({'VpcId': vpc_id}) for useridpair in
                                     ip_permission.get('UserIdGroupPairs')]
                                try:
-                                    client.authorize_security_group_ingress(GroupId=group.group_id, IpPermissions=[ips])
+                                    client.authorize_security_group_ingress(GroupId=group['GroupId'], IpPermissions=[ips])
                                except botocore.exceptions.ClientError as e:
                                    module.fail_json(
                                        msg="Unable to authorize ingress for group %s security group '%s' - %s" %
-                                            (group_id, group.group_name, e),
+                                            (group_id, group['GroupName'], e),
                                        exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
                        changed = True
                elif ip:
@ -655,22 +726,22 @@ def main():
                ip_permission = serialize_revoke(grant, rule)
                if not module.check_mode:
                    try:
-                        client.revoke_security_group_ingress(GroupId=group.group_id, IpPermissions=[ip_permission])
+                        client.revoke_security_group_ingress(GroupId=group['GroupId'], IpPermissions=[ip_permission])
                    except botocore.exceptions.ClientError as e:
                        module.fail_json(
                            msg="Unable to revoke ingress for security group '%s' - %s" %
-                                (group.group_name, e),
+                                (group['GroupName'], e),
                            exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
                changed = True

        # Manage egress rules
        groupRules = {}
-        add_rules_to_loopkup(group.ip_permissions_egress, group.id, 'out', groupRules)
+        add_rules_to_lookup(group['IpPermissionsEgress'], group['GroupId'], 'out', groupRules)
        # Now, go through all provided rules and ensure they are there.
        if rules_egress is not None:
            for rule in rules_egress:
                validate_rule(module, rule)
-                group_id, ip, ipv6, target_group_created = get_target_from_rule(module, ec2, rule, name,
+                group_id, ip, ipv6, target_group_created = get_target_from_rule(module, client, rule, name,
                                                                                group, groups, vpc_id)
                if target_group_created:
                    changed = True
@ -681,7 +752,7 @@ def main():
                    rule['to_port'] = None

                if group_id:
-                    rule_id = make_rule_key('out', rule, group.id, group_id)
+                    rule_id = make_rule_key('out', rule, group['GroupId'], group_id)
                    if rule_id in groupRules:
                        del groupRules[rule_id]
                    else:
@ -693,11 +764,11 @@ def main():
                                    [useridpair.update({'VpcId': vpc_id}) for useridpair in
                                     ip_permission.get('UserIdGroupPairs')]
                                try:
-                                    client.authorize_security_group_egress(GroupId=group.group_id, IpPermissions=[ips])
+                                    client.authorize_security_group_egress(GroupId=group['GroupId'], IpPermissions=[ips])
                                except botocore.exceptions.ClientError as e:
                                    module.fail_json(
                                        msg="Unable to authorize egress for group %s security group '%s' - %s" %
-                                            (group_id, group.group_name, e),
+                                            (group_id, group['GroupName'], e),
                                        exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
                        changed = True
                elif ip:
@ -717,7 +788,7 @@ def main():
            # when no egress rules are specified,
            # we add in a default allow all out rule, which was the
            # default behavior before egress rules were added
-            default_egress_rule = 'out--1-None-None-' + group.id + '-0.0.0.0/0'
+            default_egress_rule = 'out--1-None-None-' + group['GroupId'] + '-0.0.0.0/0'
            if default_egress_rule not in groupRules:
                if not module.check_mode:
                    ip_permission = [{'IpProtocol': '-1',
@ -725,11 +796,11 @@ def main():
                                      }
                                     ]
                    try:
-                        client.authorize_security_group_egress(GroupId=group.group_id, IpPermissions=ip_permission)
+                        client.authorize_security_group_egress(GroupId=group['GroupId'], IpPermissions=ip_permission)
                    except botocore.exceptions.ClientError as e:
                        module.fail_json(msg="Unable to authorize egress for ip %s security group '%s' - %s" %
                                             ('0.0.0.0/0',
-                                              group.group_name,
+                                              group['GroupName'],
                                              e),
                                         exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
                changed = True
@ -745,17 +816,19 @@ def main():
                    ip_permission = serialize_revoke(grant, rule)
                    if not module.check_mode:
                        try:
-                            client.revoke_security_group_egress(GroupId=group.group_id, IpPermissions=[ip_permission])
+                            client.revoke_security_group_egress(GroupId=group['GroupId'], IpPermissions=[ip_permission])
                        except botocore.exceptions.ClientError as e:
                            module.fail_json(msg="Unable to revoke egress for ip %s security group '%s' - %s" %
-                                                 (grant,
-                                                  group.group_name,
-                                                  e),
+                                                 (grant, group['GroupName'], e),
                                             exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response))
                    changed = True

    if group:
-        module.exit_json(changed=changed, group_id=group.id)
+        security_group = get_security_groups_with_backoff(client, GroupIds=[group['GroupId']])['SecurityGroups'][0]
+        security_group = camel_dict_to_snake_dict(security_group)
+        security_group['tags'] = boto3_tag_list_to_ansible_dict(security_group.get('tags', {}),
+                                                                tag_name_key_name='key', tag_value_key_name='value')
+        module.exit_json(changed=changed, **security_group)
    else:
        module.exit_json(changed=changed, group_id=None)

--- a/test/integration/targets/ec2_group/tasks/main.yml
+++ b/test/integration/targets/ec2_group/tasks/main.yml
@ -6,7 +6,7 @@
 #  - EC2_REGION -> AWS_REGION
 #

-# - include: ../../setup_ec2/tasks/common.yml module_name=ec2_group
+# - include: ../../setup_ec2/tasks/common.yml module_name: ec2_group

 - block:

@ -25,7 +25,7 @@
    # ============================================================
    - name: test failure with only name
      ec2_group:
-        name='{{ec2_group_name}}'
+        name: '{{ec2_group_name}}'
      register: result
      ignore_errors: true

@ -38,7 +38,7 @@
    # ============================================================
    - name: test failure with only description
      ec2_group:
-        description='{{ec2_group_description}}'
+        description: '{{ec2_group_description}}'
      register: result
      ignore_errors: true

@ -51,9 +51,9 @@
    # ============================================================
    - name: test failure with empty description (AWS API requires non-empty string desc)
      ec2_group:
-        name='{{ec2_group_name}}'
-        description=''
-        region='{{ec2_region}}'
+        name: '{{ec2_group_name}}'
+        description: ''
+        region: '{{ec2_region}}'
      register: result
      ignore_errors: true

@ -66,9 +66,9 @@
    # ============================================================
    - name: test valid region parameter
      ec2_group:
-        name='{{ec2_group_name}}'
-        description='{{ec2_group_description}}'
-        region='{{ec2_region}}'
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
+        region: '{{ec2_region}}'
      register: result
      ignore_errors: true

@ -81,8 +81,8 @@
    # ============================================================
    - name: test environment variable EC2_REGION
      ec2_group:
-        name='{{ec2_group_name}}'
-        description='{{ec2_group_description}}'
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
      environment:
        EC2_REGION: '{{ec2_region}}'
      register: result
@ -97,8 +97,8 @@
    # ============================================================
    - name: test invalid ec2_url parameter
      ec2_group:
-        name='{{ec2_group_name}}'
-        description='{{ec2_group_description}}'
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
      environment:
        EC2_URL: bogus.example.com
      register: result
@ -113,8 +113,8 @@
    # ============================================================
    - name: test valid ec2_url parameter
      ec2_group:
-        name='{{ec2_group_name}}'
-        description='{{ec2_group_description}}'
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
      environment:
        EC2_URL: '{{ec2_url}}'
      register: result
@ -129,8 +129,8 @@
    # ============================================================
    - name: test credentials from environment
      ec2_group:
-        name='{{ec2_group_name}}'
-        description='{{ec2_group_description}}'
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
      environment:
        EC2_REGION: '{{ec2_region}}'
        EC2_ACCESS_KEY: bogus_access_key
@ -147,11 +147,11 @@
    # ============================================================
    - name: test credential parameters
      ec2_group:
-        name='{{ec2_group_name}}'
-        description='{{ec2_group_description}}'
-        ec2_region='{{ec2_region}}'
-        ec2_access_key='bogus_access_key'
-        ec2_secret_key='bogus_secret_key'
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
+        ec2_region: '{{ec2_region}}'
+        ec2_access_key: 'bogus_access_key'
+        ec2_secret_key: 'bogus_secret_key'
      register: result
      ignore_errors: true

@ -164,25 +164,25 @@
    # ============================================================
    - name: test state=absent
      ec2_group:
-        name='{{ec2_group_name}}'
-        description='{{ec2_group_description}}'
-        ec2_region='{{ec2_region}}'
-        ec2_access_key='{{ec2_access_key}}'
-        ec2_secret_key='{{ec2_secret_key}}'
-        security_token='{{security_token}}'
-        state=absent
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
+        ec2_region: '{{ec2_region}}'
+        ec2_access_key: '{{ec2_access_key}}'
+        ec2_secret_key: '{{ec2_secret_key}}'
+        security_token: '{{security_token}}'
+        state: absent
      register: result

    # ============================================================
    - name: test state=present (expected changed=true)
      ec2_group:
-        name='{{ec2_group_name}}'
-        description='{{ec2_group_description}}'
-        ec2_region='{{ec2_region}}'
-        ec2_access_key='{{ec2_access_key}}'
-        ec2_secret_key='{{ec2_secret_key}}'
-        security_token='{{security_token}}'
-        state=present
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
+        ec2_region: '{{ec2_region}}'
+        ec2_access_key: '{{ec2_access_key}}'
+        ec2_secret_key: '{{ec2_secret_key}}'
+        security_token: '{{security_token}}'
+        state: present
      register: result

    - name: assert state=present (expected changed=true)
@ -194,13 +194,13 @@
    # ============================================================
    - name: test state=present different description raises error
      ec2_group:
-        name='{{ec2_group_name}}'
-        description='{{ec2_group_description}}CHANGED'
-        ec2_region='{{ec2_region}}'
-        ec2_access_key='{{ec2_access_key}}'
-        ec2_secret_key='{{ec2_secret_key}}'
-        security_token='{{security_token}}'
-        state=present
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}CHANGED'
+        ec2_region: '{{ec2_region}}'
+        ec2_access_key: '{{ec2_access_key}}'
+        ec2_secret_key: '{{ec2_secret_key}}'
+        security_token: '{{security_token}}'
+        state: present
      ignore_errors: true
      register: result

@ -213,13 +213,13 @@
    # ============================================================
    - name: test state=present (expected changed=false)
      ec2_group:
-        name='{{ec2_group_name}}'
-        description='{{ec2_group_description}}'
-        ec2_region='{{ec2_region}}'
-        ec2_access_key='{{ec2_access_key}}'
-        ec2_secret_key='{{ec2_secret_key}}'
-        security_token='{{security_token}}'
-        state=present
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
+        ec2_region: '{{ec2_region}}'
+        ec2_access_key: '{{ec2_access_key}}'
+        ec2_secret_key: '{{ec2_secret_key}}'
+        security_token: '{{security_token}}'
+        state: present
      register: result

    - name: assert state=present (expected changed=false)
@ -325,11 +325,36 @@
          - 'not result.changed'
          - 'result.group_id.startswith("sg-")'

+    - name: add a rule that auto creates another security group
+      ec2_group:
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
+        ec2_region: '{{ec2_region}}'
+        ec2_access_key: '{{ec2_access_key}}'
+        ec2_secret_key: '{{ec2_secret_key}}'
+        security_token: '{{security_token}}'
+        state: present
+        purge_rules: no
+        rules:
+        - proto: "tcp"
+          group_name: "{{ resource_prefix }} - Another security group"
+          group_desc: Another security group
+          ports: 7171
+      register: result
+
+    - name: check that there are now two rules
+      assert:
+        that:
+          - result.changed
+          - result.ip_permissions|length == 2
+          - result.ip_permissions[0].user_id_group_pairs or
+            result.ip_permissions[1].user_id_group_pairs
+
    # ============================================================
    - name: test state=absent (expected changed=true)
      ec2_group:
-        name='{{ec2_group_name}}'
-        state=absent
+        name: '{{ec2_group_name}}'
+        state: absent
      environment:
        EC2_REGION: '{{ec2_region}}'
        EC2_ACCESS_KEY: '{{ec2_access_key}}'
@ -343,13 +368,48 @@
           - 'result.changed'
           - 'not result.group_id'

-  always:
+    - name: create a VPC
+      ec2_vpc_net:
+        name: "{{ resource_prefix }}-vpc"
+        state: present
+        cidr_block: "10.232.232.128/26"
+        ec2_region: '{{ec2_region}}'
+        ec2_access_key: '{{ec2_access_key}}'
+        ec2_secret_key: '{{ec2_secret_key}}'
+        security_token: '{{security_token}}'
+        tags:
+          Name: "{{ resource_prefix }}-vpc"
+          Description: "Created by ansible-test"
+      register: vpc_result

-    # ============================================================
-    - name: test state=absent (expected changed=false)
+    - name: create security group in the VPC
+      ec2_group:
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
+        ec2_region: '{{ec2_region}}'
+        ec2_access_key: '{{ec2_access_key}}'
+        ec2_secret_key: '{{ec2_secret_key}}'
+        security_token: '{{security_token}}'
+        vpc_id: '{{ vpc_result.vpc.id }}'
+        state: present
+        rules:
+        - proto: "tcp"
+          from_port: 8182
+          to_port: 8182
+          cidr_ip: "1.1.1.1/32"
+      register: result
+
+    - name: assert state=present (expected changed=true)
+      assert:
+        that:
+          - 'result.changed'
+          - 'result.vpc_id == vpc_result.vpc.id'
+          - 'result.group_id.startswith("sg-")'
+
+    - name: test state=absent (expected changed=true)
      ec2_group:
-        name='{{ec2_group_name}}'
-        state=absent
+        name: '{{ec2_group_name}}'
+        state: absent
      environment:
        EC2_REGION: '{{ec2_region}}'
        EC2_ACCESS_KEY: '{{ec2_access_key}}'
@ -357,8 +417,40 @@
        EC2_SECURITY_TOKEN: '{{security_token|default("")}}'
      register: result

-    - name: assert state=absent (expected changed=false)
+    - name: assert state=absent (expected changed=true)
      assert:
        that:
-           - 'not result.changed'
+           - 'result.changed'
           - 'not result.group_id'
+
+  always:
+
+    # ============================================================
+    - name: tidy up security group
+      ec2_group:
+        name: '{{ec2_group_name}}'
+        state: absent
+      environment:
+        EC2_REGION: '{{ec2_region}}'
+        EC2_ACCESS_KEY: '{{ec2_access_key}}'
+        EC2_SECRET_KEY: '{{ec2_secret_key}}'
+        EC2_SECURITY_TOKEN: '{{security_token|default("")}}'
+
+    - name: tidy up automatically created SG
+      ec2_group:
+        name: "{{ resource_prefix }} - Another security group"
+        state: absent
+        ec2_region: '{{ec2_region}}'
+        ec2_access_key: '{{ec2_access_key}}'
+        ec2_secret_key: '{{ec2_secret_key}}'
+        security_token: '{{security_token}}'
+
+    - name: tidy up VPC
+      ec2_vpc_net:
+        name: "{{ resource_prefix }}-vpc"
+        state: absent
+        cidr_block: "10.232.232.128/26"
+        ec2_region: '{{ec2_region}}'
+        ec2_access_key: '{{ec2_access_key}}'
+        ec2_secret_key: '{{ec2_secret_key}}'
+        security_token: '{{security_token}}'