From 3cb67e4ece6b25f355903d91abbb52fc51ad3697 Mon Sep 17 00:00:00 2001
From: Mark Chappell <mchappel@redhat.com>
Date: Fri, 17 Sep 2021 20:38:34 +0200
Subject: [PATCH 1/5] Migrate elasticache_subnet_group to boto3

---
 .../723-elasticache_subnet_group-boto3.yml    |   2 +
 plugins/modules/elasticache_subnet_group.py   | 196 ++++++++++++------
 2 files changed, 129 insertions(+), 69 deletions(-)
 create mode 100644 changelogs/fragments/723-elasticache_subnet_group-boto3.yml

diff --git a/changelogs/fragments/723-elasticache_subnet_group-boto3.yml b/changelogs/fragments/723-elasticache_subnet_group-boto3.yml
new file mode 100644
index 00000000000..a54e451ec7b
--- /dev/null
+++ b/changelogs/fragments/723-elasticache_subnet_group-boto3.yml
@@ -0,0 +1,2 @@
+minor_changes:
+- elasticache_subnet_group - module migrated to boto3 AWS SDK (https://github.com/ansible-collections/community.aws/pull/723).
diff --git a/plugins/modules/elasticache_subnet_group.py b/plugins/modules/elasticache_subnet_group.py
index 44a3e39ae6f..f982a7a253c 100644
--- a/plugins/modules/elasticache_subnet_group.py
+++ b/plugins/modules/elasticache_subnet_group.py
@@ -12,34 +12,35 @@
 version_added: 1.0.0
 short_description: manage ElastiCache subnet groups
 description:
-     - Creates, modifies, and deletes ElastiCache subnet groups. This module has a dependency on python-boto >= 2.5.
+     - Creates, modifies, and deletes ElastiCache subnet groups.
 options:
   state:
     description:
       - Specifies whether the subnet should be present or absent.
-    required: true
     choices: [ 'present' , 'absent' ]
+    default: 'present'
     type: str
   name:
     description:
       - Database subnet group identifier.
+      - This value is automatically converted to lowercase.
     required: true
     type: str
   description:
     description:
-      - ElastiCache subnet group description. Only set when a new group is added.
+      - ElastiCache subnet group description.
+      - When not provided defaults to I(name) on subnet group creation.
     type: str
   subnets:
     description:
       - List of subnet IDs that make up the ElastiCache subnet group.
     type: list
     elements: str
-author: "Tim Mahoney (@timmahoney)"
+author:
+  - "Tim Mahoney (@timmahoney)"
 extends_documentation_fragment:
-- amazon.aws.aws
-- amazon.aws.ec2
-requirements:
-- boto >= 2.49.0
+  - amazon.aws.aws
+  - amazon.aws.ec2
 '''
 
 EXAMPLES = r'''
@@ -59,86 +60,143 @@
 '''
 
 try:
-    import boto
-    from boto.elasticache import connect_to_region
-    from boto.exception import BotoServerError
+    import botocore
 except ImportError:
-    pass  # Handled by HAS_BOTO
+    pass  # Handled by AnsibleAWSModule
+
+from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict
 
-from ansible.module_utils._text import to_native
 from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule
-from ansible_collections.amazon.aws.plugins.module_utils.ec2 import HAS_BOTO
-from ansible_collections.amazon.aws.plugins.module_utils.ec2 import get_aws_connection_info
+from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry
+
+
+def get_subnet_group(name):
+    try:
+        groups = client.describe_cache_subnet_groups(
+            aws_retry=True,
+            CacheSubnetGroupName=name,
+        )['CacheSubnetGroups']
+    except is_boto3_error_code('CacheSubnetGroupNotFoundFault'):
+        return None
+    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:  # pylint: disable=duplicate-except
+        module.fail_json_aws(e, msg="Failed to describe subnet group")
+
+    if not groups:
+        return None
+
+    if len(groups) > 1:
+        module.fail_aws(
+            msg="Found multiple matches for subnet group",
+            cache_subnet_groups=camel_dict_to_snake_dict(groups),
+        )
+
+    subnet_group = camel_dict_to_snake_dict(groups[0])
+
+    subnet_group['name'] = subnet_group['cache_subnet_group_name']
+    subnet_group['description'] = subnet_group['cache_subnet_group_description']
+
+    subnet_ids = list(s['subnet_identifier'] for s in subnet_group['subnets'])
+    subnet_group['subnet_ids'] = subnet_ids
+
+    return subnet_group
+
+
+def create_subnet_group(name, description, subnets):
+    try:
+        if not description:
+            description = name
+        if not subnets:
+            subnets = []
+        client.create_cache_subnet_group(
+            aws_retry=True,
+            CacheSubnetGroupName=name,
+            CacheSubnetGroupDescription=description,
+            SubnetIds=subnets,
+        )
+        return True
+    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+        module.fail_json_aws(e, msg="Failed to create subnet group")
+
+
+def update_subnet_group(subnet_group, name, description, subnets):
+    update_params = dict()
+    if description and subnet_group['description'] != description:
+        update_params['CacheSubnetGroupDescription'] = description
+    if subnets:
+        old_subnets = set(subnet_group['subnet_ids'])
+        new_subnets = set(subnets)
+        if old_subnets != new_subnets:
+            update_params['SubnetIds'] = list(subnets)
+
+    if not update_params:
+        return False
+
+    try:
+        client.modify_cache_subnet_group(
+            aws_retry=True,
+            CacheSubnetGroupName=name,
+            **update_params,
+        )
+    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+        module.fail_json_aws(e, msg="Failed to update subnet group")
+
+    return True
+
+
+def delete_subnet_group(name):
+    try:
+        client.delete_cache_subnet_group(
+            aws_retry=True,
+            CacheSubnetGroupName=name,
+        )
+        return True
+    except is_boto3_error_code('CacheSubnetGroupNotFoundFault'):
+        # AWS is "eventually consistent", cope with the race conditions where
+        # deletion hadn't completed when we ran describe
+        return False
+    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:  # pylint: disable=duplicate-except
+        module.fail_json_aws(e, msg="Failed to delete subnet group")
 
 
 def main():
     argument_spec = dict(
-        state=dict(required=True, choices=['present', 'absent']),
+        state=dict(default='present', choices=['present', 'absent']),
         name=dict(required=True),
         description=dict(required=False),
         subnets=dict(required=False, type='list', elements='str'),
     )
-    module = AnsibleAWSModule(argument_spec=argument_spec, check_boto3=False)
 
-    if not HAS_BOTO:
-        module.fail_json(msg='boto required for this module')
+    global module
+    global client
 
-    state = module.params.get('state')
-    group_name = module.params.get('name').lower()
-    group_description = module.params.get('description')
-    group_subnets = module.params.get('subnets') or {}
+    module = AnsibleAWSModule(argument_spec=argument_spec)
 
-    if state == 'present':
-        for required in ['name', 'description', 'subnets']:
-            if not module.params.get(required):
-                module.fail_json(msg=str("Parameter %s required for state='present'" % required))
-    else:
-        for not_allowed in ['description', 'subnets']:
-            if module.params.get(not_allowed):
-                module.fail_json(msg=str("Parameter %s not allowed for state='absent'" % not_allowed))
-
-    # Retrieve any AWS settings from the environment.
-    region, ec2_url, aws_connect_kwargs = get_aws_connection_info(module)
+    state = module.params.get('state')
+    name = module.params.get('name').lower()
+    description = module.params.get('description')
+    subnets = module.params.get('subnets')
 
-    if not region:
-        module.fail_json(msg=str("Either region or AWS_REGION or EC2_REGION environment variable or boto config aws_region or ec2_region must be set."))
+    client = module.client('elasticache', retry_decorator=AWSRetry.jittered_backoff())
 
-    """Get an elasticache connection"""
-    try:
-        conn = connect_to_region(region_name=region, **aws_connect_kwargs)
-    except boto.exception.NoAuthHandlerFound as e:
-        module.fail_json(msg=to_native(e))
+    subnet_group = get_subnet_group(name)
+    changed = False
 
-    try:
-        changed = False
-        exists = False
-
-        try:
-            matching_groups = conn.describe_cache_subnet_groups(group_name, max_records=100)
-            exists = len(matching_groups) > 0
-        except BotoServerError as e:
-            if e.error_code != 'CacheSubnetGroupNotFoundFault':
-                module.fail_json(msg=e.error_message)
-
-        if state == 'absent':
-            if exists:
-                conn.delete_cache_subnet_group(group_name)
-                changed = True
-        else:
-            if not exists:
-                new_group = conn.create_cache_subnet_group(group_name, cache_subnet_group_description=group_description, subnet_ids=group_subnets)
-                changed = True
-            else:
-                changed_group = conn.modify_cache_subnet_group(group_name, cache_subnet_group_description=group_description, subnet_ids=group_subnets)
-                changed = True
-
-    except BotoServerError as e:
-        if e.error_message != 'No modifications were requested.':
-            module.fail_json(msg=e.error_message)
+    if state == 'present':
+        if not subnet_group:
+            result = create_subnet_group(name, description, subnets)
+            changed |= result
         else:
-            changed = False
+            result = update_subnet_group(subnet_group, name, description, subnets)
+            changed |= result
+        subnet_group = get_subnet_group(name)
+    else:
+        if subnet_group:
+            result = delete_subnet_group(name)
+            changed |= result
+        subnet_group = None
 
-    module.exit_json(changed=changed)
+    module.exit_json(changed=changed, cache_subnet_group=subnet_group)
 
 
 if __name__ == '__main__':

From 24c82110fc9c713f2845ceb3c6beef7b9eebcbc6 Mon Sep 17 00:00:00 2001
From: Mark Chappell <mchappel@redhat.com>
Date: Fri, 17 Sep 2021 22:06:13 +0200
Subject: [PATCH 2/5] Add documentation and tests for return values

---
 .../723-elasticache_subnet_group-boto3.yml    |   1 +
 plugins/modules/elasticache_subnet_group.py   |  36 ++++++
 .../elasticache_subnet_group/tasks/main.yml   | 120 +++++++++++++-----
 3 files changed, 127 insertions(+), 30 deletions(-)

diff --git a/changelogs/fragments/723-elasticache_subnet_group-boto3.yml b/changelogs/fragments/723-elasticache_subnet_group-boto3.yml
index a54e451ec7b..91171b03e54 100644
--- a/changelogs/fragments/723-elasticache_subnet_group-boto3.yml
+++ b/changelogs/fragments/723-elasticache_subnet_group-boto3.yml
@@ -1,2 +1,3 @@
 minor_changes:
 - elasticache_subnet_group - module migrated to boto3 AWS SDK (https://github.com/ansible-collections/community.aws/pull/723).
+- elasticache_subnet_group - add return values (https://github.com/ansible-collections/community.aws/pull/723).
diff --git a/plugins/modules/elasticache_subnet_group.py b/plugins/modules/elasticache_subnet_group.py
index f982a7a253c..c5ef1d0f606 100644
--- a/plugins/modules/elasticache_subnet_group.py
+++ b/plugins/modules/elasticache_subnet_group.py
@@ -59,6 +59,42 @@
     name: norwegian-blue
 '''
 
+RETURN = r'''
+cache_subnet_group:
+  description: Description of the Elasticache Subnet Group.
+  returned: always
+  type: dict
+  contains:
+    arn:
+      description: The Amazon Resource Name (ARN) of the cache subnet group.
+      returned: when the subnet group exists
+      type: str
+      sample: arn:aws:elasticache:us-east-1:012345678901:subnetgroup:norwegian-blue
+    description:
+      description: The description of the cache subnet group.
+      returned: when the cache subnet group exists
+      type: str
+      sample: My Fancy Ex Parrot Subnet Group
+    name:
+      description: The name of the cache subnet group.
+      returned: when the cache subnet group exists
+      type: str
+      sample: norwegian-blue
+    vpc_id:
+      description: The VPC ID of the cache subnet group.
+      returned: when the cache subnet group exists
+      type: str
+      sample: norwegian-blue
+    subnet_ids:
+      description: The IDs of the subnets beloging to the cache subnet group.
+      returned: when the cache subnet group exists
+      type: list
+      elements: str
+      sample:
+        - subnet-aaaaaaaa
+        - subnet-bbbbbbbb
+'''
+
 try:
     import botocore
 except ImportError:
diff --git a/tests/integration/targets/elasticache_subnet_group/tasks/main.yml b/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
index 3f2774b2b17..5f938966e4b 100644
--- a/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
+++ b/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
@@ -72,11 +72,21 @@
         that:
           - create_group is successful
           - create_group is changed
-          #- '"group" in create_group'
-          #- '"name" in create_group.group'
-          #- '"vpc_id" in create_group.group'
-          #- create_group.group.name == group_name
-          #- create_group.group.vpc_id == vpc_id
+          - '"cache_subnet_group" in create_group'
+          - '"arn" in create_group.cache_subnet_group'
+          - '"description" in create_group.cache_subnet_group'
+          - '"name" in create_group.cache_subnet_group'
+          - '"subnet_ids" in create_group.cache_subnet_group'
+          - '"vpc_id" in create_group.cache_subnet_group'
+          - create_group.cache_subnet_group.description == description_default
+          - create_group.cache_subnet_group.name == group_name
+          - subnet_id_a in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_b in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_c not in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_d not in create_group.cache_subnet_group.subnet_ids
+          - create_group.cache_subnet_group.vpc_id == vpc_id
+          - create_group.cache_subnet_group.arn.startswith('arn:')
+          - create_group.cache_subnet_group.arn.endswith(group_name)
 
     - name: Create Subnet Group - idempotency
       elasticache_subnet_group:
@@ -93,11 +103,21 @@
         that:
           - create_group is successful
           - create_group is not changed
-          #- '"group" in create_group'
-          #- '"name" in create_group.group'
-          #- '"vpc_id" in create_group.group'
-          #- create_group.group.name == group_name
-          #- create_group.group.vpc_id == vpc_id
+          - '"cache_subnet_group" in create_group'
+          - '"arn" in create_group.cache_subnet_group'
+          - '"description" in create_group.cache_subnet_group'
+          - '"name" in create_group.cache_subnet_group'
+          - '"subnet_ids" in create_group.cache_subnet_group'
+          - '"vpc_id" in create_group.cache_subnet_group'
+          - create_group.cache_subnet_group.description == description_default
+          - create_group.cache_subnet_group.name == group_name
+          - subnet_id_a in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_b in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_c not in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_d not in create_group.cache_subnet_group.subnet_ids
+          - create_group.cache_subnet_group.vpc_id == vpc_id
+          - create_group.cache_subnet_group.arn.startswith('arn:')
+          - create_group.cache_subnet_group.arn.endswith(group_name)
 
     # ============================================================
 
@@ -116,11 +136,21 @@
         that:
           - update_description is successful
           - update_description is changed
-          #- '"group" in update_description'
-          #- '"name" in update_description.group'
-          #- '"vpc_id" in update_description.group'
-          #- update_description.group.name == group_name
-          #- update_description.group.vpc_id == vpc_id
+          - '"cache_subnet_group" in update_description'
+          - '"arn" in update_description.cache_subnet_group'
+          - '"description" in update_description.cache_subnet_group'
+          - '"name" in update_description.cache_subnet_group'
+          - '"subnet_ids" in update_description.cache_subnet_group'
+          - '"vpc_id" in update_description.cache_subnet_group'
+          - update_description.cache_subnet_group.description == description_updated
+          - update_description.cache_subnet_group.name == group_name
+          - subnet_id_a in update_description.cache_subnet_group.subnet_ids
+          - subnet_id_b in update_description.cache_subnet_group.subnet_ids
+          - subnet_id_c not in update_description.cache_subnet_group.subnet_ids
+          - subnet_id_d not in update_description.cache_subnet_group.subnet_ids
+          - update_description.cache_subnet_group.vpc_id == vpc_id
+          - update_description.cache_subnet_group.arn.startswith('arn:')
+          - update_description.cache_subnet_group.arn.endswith(group_name)
 
     - name: Update Subnet Group Description - idempotency
       elasticache_subnet_group:
@@ -137,11 +167,21 @@
         that:
           - update_description is successful
           - update_description is not changed
-          #- '"group" in update_description'
-          #- '"name" in update_description.group'
-          #- '"vpc_id" in update_description.group'
-          #- update_description.group.name == group_name
-          #- update_description.group.vpc_id == vpc_id
+          - '"cache_subnet_group" in update_description'
+          - '"arn" in update_description.cache_subnet_group'
+          - '"description" in update_description.cache_subnet_group'
+          - '"name" in update_description.cache_subnet_group'
+          - '"subnet_ids" in update_description.cache_subnet_group'
+          - '"vpc_id" in update_description.cache_subnet_group'
+          - update_description.cache_subnet_group.description == description_updated
+          - update_description.cache_subnet_group.name == group_name
+          - subnet_id_a in update_description.cache_subnet_group.subnet_ids
+          - subnet_id_b in update_description.cache_subnet_group.subnet_ids
+          - subnet_id_c not in update_description.cache_subnet_group.subnet_ids
+          - subnet_id_d not in update_description.cache_subnet_group.subnet_ids
+          - update_description.cache_subnet_group.vpc_id == vpc_id
+          - update_description.cache_subnet_group.arn.startswith('arn:')
+          - update_description.cache_subnet_group.arn.endswith(group_name)
 
     # ============================================================
 
@@ -160,11 +200,21 @@
         that:
           - update_subnets is successful
           - update_subnets is changed
-          #- '"group" in update_subnets'
-          #- '"name" in update_subnets.group'
-          #- '"vpc_id" in update_subnets.group'
-          #- update_subnets.group.name == group_name
-          #- update_subnets.group.vpc_id == vpc_id
+          - '"cache_subnet_group" in update_subnets'
+          - '"arn" in update_subnets.cache_subnet_group'
+          - '"description" in update_subnets.cache_subnet_group'
+          - '"name" in update_subnets.cache_subnet_group'
+          - '"subnet_ids" in update_subnets.cache_subnet_group'
+          - '"vpc_id" in update_subnets.cache_subnet_group'
+          - update_subnets.cache_subnet_group.description == description_updated
+          - update_subnets.cache_subnet_group.name == group_name
+          - subnet_id_a not in update_subnets.cache_subnet_group.subnet_ids
+          - subnet_id_b not in update_subnets.cache_subnet_group.subnet_ids
+          - subnet_id_c in update_subnets.cache_subnet_group.subnet_ids
+          - subnet_id_d in update_subnets.cache_subnet_group.subnet_ids
+          - update_subnets.cache_subnet_group.vpc_id == vpc_id
+          - update_subnets.cache_subnet_group.arn.startswith('arn:')
+          - update_subnets.cache_subnet_group.arn.endswith(group_name)
 
     - name: Update Subnet Group subnets - idempotency
       elasticache_subnet_group:
@@ -181,11 +231,21 @@
         that:
           - update_subnets is successful
           - update_subnets is not changed
-          #- '"group" in update_subnets'
-          #- '"name" in update_subnets.group'
-          #- '"vpc_id" in update_subnets.group'
-          #- update_subnets.group.name == group_name
-          #- update_subnets.group.vpc_id == vpc_id
+          - '"cache_subnet_group" in update_subnets'
+          - '"arn" in update_subnets.cache_subnet_group'
+          - '"description" in update_subnets.cache_subnet_group'
+          - '"name" in update_subnets.cache_subnet_group'
+          - '"subnet_ids" in update_subnets.cache_subnet_group'
+          - '"vpc_id" in update_subnets.cache_subnet_group'
+          - update_subnets.cache_subnet_group.description == description_updated
+          - update_subnets.cache_subnet_group.name == group_name
+          - subnet_id_a not in update_subnets.cache_subnet_group.subnet_ids
+          - subnet_id_b not in update_subnets.cache_subnet_group.subnet_ids
+          - subnet_id_c in update_subnets.cache_subnet_group.subnet_ids
+          - subnet_id_d in update_subnets.cache_subnet_group.subnet_ids
+          - update_subnets.cache_subnet_group.vpc_id == vpc_id
+          - update_subnets.cache_subnet_group.arn.startswith('arn:')
+          - update_subnets.cache_subnet_group.arn.endswith(group_name)
 
     # ============================================================
 

From e6804cefe6650b559af56b72dc304ebc9c82b66b Mon Sep 17 00:00:00 2001
From: Mark Chappell <mchappel@redhat.com>
Date: Fri, 17 Sep 2021 20:48:03 +0200
Subject: [PATCH 3/5] Add support for check_mode

---
 .../723-elasticache_subnet_group-boto3.yml    |   1 +
 plugins/modules/elasticache_subnet_group.py   |  16 +-
 .../elasticache_subnet_group/tasks/main.yml   | 150 +++++++++++++++++-
 3 files changed, 158 insertions(+), 9 deletions(-)

diff --git a/changelogs/fragments/723-elasticache_subnet_group-boto3.yml b/changelogs/fragments/723-elasticache_subnet_group-boto3.yml
index 91171b03e54..fc291737da9 100644
--- a/changelogs/fragments/723-elasticache_subnet_group-boto3.yml
+++ b/changelogs/fragments/723-elasticache_subnet_group-boto3.yml
@@ -1,3 +1,4 @@
 minor_changes:
 - elasticache_subnet_group - module migrated to boto3 AWS SDK (https://github.com/ansible-collections/community.aws/pull/723).
 - elasticache_subnet_group - add return values (https://github.com/ansible-collections/community.aws/pull/723).
+- elasticache_subnet_group - add support for check_mode (https://github.com/ansible-collections/community.aws/pull/723).
diff --git a/plugins/modules/elasticache_subnet_group.py b/plugins/modules/elasticache_subnet_group.py
index c5ef1d0f606..5e813b64ad1 100644
--- a/plugins/modules/elasticache_subnet_group.py
+++ b/plugins/modules/elasticache_subnet_group.py
@@ -139,6 +139,10 @@ def get_subnet_group(name):
 
 
 def create_subnet_group(name, description, subnets):
+
+    if module.check_mode:
+        return True
+
     try:
         if not description:
             description = name
@@ -168,6 +172,9 @@ def update_subnet_group(subnet_group, name, description, subnets):
     if not update_params:
         return False
 
+    if module.check_mode:
+        return True
+
     try:
         client.modify_cache_subnet_group(
             aws_retry=True,
@@ -181,6 +188,10 @@ def update_subnet_group(subnet_group, name, description, subnets):
 
 
 def delete_subnet_group(name):
+
+    if module.check_mode:
+        return True
+
     try:
         client.delete_cache_subnet_group(
             aws_retry=True,
@@ -206,7 +217,10 @@ def main():
     global module
     global client
 
-    module = AnsibleAWSModule(argument_spec=argument_spec)
+    module = AnsibleAWSModule(
+        argument_spec=argument_spec,
+        supports_check_mode=True,
+    )
 
     state = module.params.get('state')
     name = module.params.get('name').lower()
diff --git a/tests/integration/targets/elasticache_subnet_group/tasks/main.yml b/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
index 5f938966e4b..cc82948a1ce 100644
--- a/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
+++ b/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
@@ -57,6 +57,23 @@
         subnet_id_d: '{{ vpc_subnet_create.results[3].subnet.id }}'
     # ============================================================
 
+    - name: Create Subnet Group - check_mode
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        description: '{{ description_default }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+        - '{{ subnet_id_b }}'
+      check_mode: True
+      register: create_group
+
+    - name: Check result - Create Subnet Group - check_mode
+      assert:
+        that:
+          - create_group is successful
+          - create_group is changed
+
     - name: Create Subnet Group
       elasticache_subnet_group:
         state: present
@@ -88,6 +105,23 @@
           - create_group.cache_subnet_group.arn.startswith('arn:')
           - create_group.cache_subnet_group.arn.endswith(group_name)
 
+    - name: Create Subnet Group - idempotency - check_mode
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        description: '{{ description_default }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+        - '{{ subnet_id_b }}'
+      check_mode: True
+      register: create_group
+
+    - name: Check result - Create Subnet Group - idempotency - check_mode
+      assert:
+        that:
+          - create_group is successful
+          - create_group is not changed
+
     - name: Create Subnet Group - idempotency
       elasticache_subnet_group:
         state: present
@@ -121,14 +155,33 @@
 
     # ============================================================
 
+    - name: Update Subnet Group Description - check_mode
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        description: '{{ description_updated }}'
+        ## No longer mandatory
+        # subnets:
+        # - '{{ subnet_id_a }}'
+        # - '{{ subnet_id_b }}'
+      check_mode: True
+      register: update_description
+
+    - name: Check result - Update Subnet Group Description - check_mode
+      assert:
+        that:
+          - update_description is successful
+          - update_description is changed
+
     - name: Update Subnet Group Description
       elasticache_subnet_group:
         state: present
         name: '{{ group_name }}'
         description: '{{ description_updated }}'
-        subnets:
-        - '{{ subnet_id_a }}'
-        - '{{ subnet_id_b }}'
+        ## No longer mandatory
+        # subnets:
+        # - '{{ subnet_id_a }}'
+        # - '{{ subnet_id_b }}'
       register: update_description
 
     - name: Check result - Update Subnet Group Description
@@ -152,14 +205,33 @@
           - update_description.cache_subnet_group.arn.startswith('arn:')
           - update_description.cache_subnet_group.arn.endswith(group_name)
 
+    - name: Update Subnet Group Description - idempotency - check_mode
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        description: '{{ description_updated }}'
+        ## No longer mandatory
+        # subnets:
+        # - '{{ subnet_id_a }}'
+        # - '{{ subnet_id_b }}'
+      check_mode: True
+      register: update_description
+
+    - name: Check result - Update Subnet Group Description - idempotency - check_mode
+      assert:
+        that:
+          - update_description is successful
+          - update_description is not changed
+
     - name: Update Subnet Group Description - idempotency
       elasticache_subnet_group:
         state: present
         name: '{{ group_name }}'
         description: '{{ description_updated }}'
-        subnets:
-        - '{{ subnet_id_a }}'
-        - '{{ subnet_id_b }}'
+        ## No longer mandatory
+        # subnets:
+        # - '{{ subnet_id_a }}'
+        # - '{{ subnet_id_b }}'
       register: update_description
 
     - name: Check result - Update Subnet Group Description - idempotency
@@ -185,11 +257,30 @@
 
     # ============================================================
 
+    - name: Update Subnet Group subnets - check_mode
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        ## No longer mandatory
+        # description: '{{ description_updated }}'
+        subnets:
+        - '{{ subnet_id_c }}'
+        - '{{ subnet_id_d }}'
+      check_mode: True
+      register: update_subnets
+
+    - name: Check result - Update Subnet Group subnets - check_mode
+      assert:
+        that:
+          - update_subnets is successful
+          - update_subnets is changed
+
     - name: Update Subnet Group subnets
       elasticache_subnet_group:
         state: present
         name: '{{ group_name }}'
-        description: '{{ description_updated }}'
+        ## No longer mandatory
+        # description: '{{ description_updated }}'
         subnets:
         - '{{ subnet_id_c }}'
         - '{{ subnet_id_d }}'
@@ -216,11 +307,30 @@
           - update_subnets.cache_subnet_group.arn.startswith('arn:')
           - update_subnets.cache_subnet_group.arn.endswith(group_name)
 
+    - name: Update Subnet Group subnets - idempotency - check_mode
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        ## No longer mandatory
+        # description: '{{ description_updated }}'
+        subnets:
+        - '{{ subnet_id_c }}'
+        - '{{ subnet_id_d }}'
+      check_mode: True
+      register: update_subnets
+
+    - name: Check result - Update Subnet Group subnets - idempotency - check_mode
+      assert:
+        that:
+          - update_subnets is successful
+          - update_subnets is not changed
+
     - name: Update Subnet Group subnets - idempotency
       elasticache_subnet_group:
         state: present
         name: '{{ group_name }}'
-        description: '{{ description_updated }}'
+        ## No longer mandatory
+        # description: '{{ description_updated }}'
         subnets:
         - '{{ subnet_id_c }}'
         - '{{ subnet_id_d }}'
@@ -249,6 +359,18 @@
 
     # ============================================================
 
+    - name: Delete Subnet Group - check_mode
+      elasticache_subnet_group:
+        state: absent
+        name: '{{ group_name }}'
+      check_mode: True
+      register: delete_group
+
+    - name: Check result - Delete Subnet Group - check_mode
+      assert:
+        that:
+          - delete_group is changed
+
     - name: Delete Subnet Group
       elasticache_subnet_group:
         state: absent
@@ -260,6 +382,18 @@
         that:
           - delete_group is changed
 
+    - name: Delete Subnet Group - idempotency - check_mode
+      elasticache_subnet_group:
+        state: absent
+        name: '{{ group_name }}'
+      check_mode: True
+      register: delete_group
+
+    - name: Check result - Delete Subnet Group - idempotency - check_mode
+      assert:
+        that:
+          - delete_group is not changed
+
     - name: Delete Subnet Group - idempotency
       elasticache_subnet_group:
         state: absent

From 8db0ada7873c90f2adb3e4dcf6fbce2831d0562c Mon Sep 17 00:00:00 2001
From: Mark Chappell <mchappel@redhat.com>
Date: Sat, 18 Sep 2021 08:35:38 +0200
Subject: [PATCH 4/5] Fail cleanly if no subnets are provided on creation.

---
 plugins/modules/elasticache_subnet_group.py   |  6 ++-
 .../elasticache_subnet_group/tasks/main.yml   | 43 +++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/plugins/modules/elasticache_subnet_group.py b/plugins/modules/elasticache_subnet_group.py
index 5e813b64ad1..eda678205d0 100644
--- a/plugins/modules/elasticache_subnet_group.py
+++ b/plugins/modules/elasticache_subnet_group.py
@@ -34,6 +34,7 @@
   subnets:
     description:
       - List of subnet IDs that make up the ElastiCache subnet group.
+      - At least one subnet must be provided when creating an ElastiCache subnet group.
     type: list
     elements: str
 author:
@@ -140,14 +141,15 @@ def get_subnet_group(name):
 
 def create_subnet_group(name, description, subnets):
 
+    if not subnets:
+        module.fail_json(msg='At least one subnet must be provided when creating a subnet group')
+
     if module.check_mode:
         return True
 
     try:
         if not description:
             description = name
-        if not subnets:
-            subnets = []
         client.create_cache_subnet_group(
             aws_retry=True,
             CacheSubnetGroupName=name,
diff --git a/tests/integration/targets/elasticache_subnet_group/tasks/main.yml b/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
index cc82948a1ce..ca94dc11f2c 100644
--- a/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
+++ b/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
@@ -13,6 +13,48 @@
       security_token: '{{ security_token | default(omit) }}'
       region: '{{ aws_region }}'
   block:
+
+    # ============================================================
+
+    - name: Create Subnet Group with no subnets - check_mode
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+      check_mode: True
+      register: create_group
+      ignore_errors: True
+
+    - name: Check result - Create Subnet Group with no subnets - check_mode
+      assert:
+        that:
+          - create_group is failed
+          # Check we caught the issue before trying to create
+          - '"CreateCacheSubnetGroup" not in create_group.resource_actions'
+          # Check that we don't refer to the boto3 parameter
+          - '"SubnetIds" not in create_group.msg'
+          # Loosely check the message
+          - '"subnet" in create_group.msg'
+          - '"At least" in create_group.msg'
+
+    - name: Create Subnet Group with no subnets
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+      register: create_group
+      ignore_errors: True
+
+    - name: Check result - Create Subnet Group with no subnets
+      assert:
+        that:
+          - create_group is failed
+          # Check we caught the issue before trying to create
+          - '"CreateCacheSubnetGroup" not in create_group.resource_actions'
+          # Check that we don't refer to the boto3 parameter
+          - '"SubnetIds" not in create_group.msg'
+          # Loosely check the message
+          - '"subnet" in create_group.msg'
+          - '"At least" in create_group.msg'
+
     # ============================================================
     # Setup infra needed for tests
     - name: create a VPC
@@ -55,6 +97,7 @@
         subnet_id_b: '{{ vpc_subnet_create.results[1].subnet.id }}'
         subnet_id_c: '{{ vpc_subnet_create.results[2].subnet.id }}'
         subnet_id_d: '{{ vpc_subnet_create.results[3].subnet.id }}'
+
     # ============================================================
 
     - name: Create Subnet Group - check_mode

From d779a7bc601dbd943a4466cea6a1d0b1e841f875 Mon Sep 17 00:00:00 2001
From: Mark Chappell <mchappel@redhat.com>
Date: Sat, 18 Sep 2021 08:36:41 +0200
Subject: [PATCH 5/5] Test creation with no description provided

---
 .../elasticache_subnet_group/tasks/main.yml   | 201 ++++++++++++++++++
 1 file changed, 201 insertions(+)

diff --git a/tests/integration/targets/elasticache_subnet_group/tasks/main.yml b/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
index ca94dc11f2c..5814f9dc90d 100644
--- a/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
+++ b/tests/integration/targets/elasticache_subnet_group/tasks/main.yml
@@ -448,6 +448,207 @@
         that:
           - delete_group is not changed
 
+    # ============================================================
+
+    - name: Create minimal Subnet Group - check_mode
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+      check_mode: True
+      register: create_group
+
+    - name: Check result - Create minimal Subnet Group - check_mode
+      assert:
+        that:
+          - create_group is successful
+          - create_group is changed
+
+    - name: Create minimal Subnet Group
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+      register: create_group
+
+    - name: Check result - Create minimal Subnet Group
+      assert:
+        that:
+          - create_group is successful
+          - create_group is changed
+          - '"cache_subnet_group" in create_group'
+          - '"arn" in create_group.cache_subnet_group'
+          - '"description" in create_group.cache_subnet_group'
+          - '"name" in create_group.cache_subnet_group'
+          - '"subnet_ids" in create_group.cache_subnet_group'
+          - '"vpc_id" in create_group.cache_subnet_group'
+          - create_group.cache_subnet_group.description == group_name
+          - create_group.cache_subnet_group.name == group_name
+          - subnet_id_a in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_b not in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_c not in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_d not in create_group.cache_subnet_group.subnet_ids
+          - create_group.cache_subnet_group.vpc_id == vpc_id
+          - create_group.cache_subnet_group.arn.startswith('arn:')
+          - create_group.cache_subnet_group.arn.endswith(group_name)
+
+    - name: Create minimal Subnet Group - idempotency - check_mode
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+      check_mode: True
+      register: create_group
+
+    - name: Check result - Create minimal Subnet Group - idempotency - check_mode
+      assert:
+        that:
+          - create_group is successful
+          - create_group is not changed
+
+    - name: Create minimal Subnet Group - idempotency
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+      register: create_group
+
+    - name: Check result - Create minimal Subnet Group - idempotency
+      assert:
+        that:
+          - create_group is successful
+          - create_group is not changed
+          - '"cache_subnet_group" in create_group'
+          - '"arn" in create_group.cache_subnet_group'
+          - '"description" in create_group.cache_subnet_group'
+          - '"name" in create_group.cache_subnet_group'
+          - '"subnet_ids" in create_group.cache_subnet_group'
+          - '"vpc_id" in create_group.cache_subnet_group'
+          - create_group.cache_subnet_group.description == group_name
+          - create_group.cache_subnet_group.name == group_name
+          - subnet_id_a in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_b not in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_c not in create_group.cache_subnet_group.subnet_ids
+          - subnet_id_d not in create_group.cache_subnet_group.subnet_ids
+          - create_group.cache_subnet_group.vpc_id == vpc_id
+          - create_group.cache_subnet_group.arn.startswith('arn:')
+          - create_group.cache_subnet_group.arn.endswith(group_name)
+
+    # ============================================================
+
+    - name: Full Update Subnet Group - check_mode
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        description: '{{ description_updated }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+        - '{{ subnet_id_b }}'
+      check_mode: True
+      register: update_complex
+
+    - name: Check result - Full Update Subnet Group - check_mode
+      assert:
+        that:
+          - update_complex is successful
+          - update_complex is changed
+
+    - name: Update Subnet Group
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        description: '{{ description_updated }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+        - '{{ subnet_id_b }}'
+      register: update_complex
+
+    - name: Check result - Full Update Subnet Group
+      assert:
+        that:
+          - update_complex is successful
+          - update_complex is changed
+          - '"cache_subnet_group" in update_complex'
+          - '"arn" in update_complex.cache_subnet_group'
+          - '"description" in update_complex.cache_subnet_group'
+          - '"name" in update_complex.cache_subnet_group'
+          - '"subnet_ids" in update_complex.cache_subnet_group'
+          - '"vpc_id" in update_complex.cache_subnet_group'
+          - update_complex.cache_subnet_group.description == description_updated
+          - update_complex.cache_subnet_group.name == group_name
+          - subnet_id_a in update_complex.cache_subnet_group.subnet_ids
+          - subnet_id_b in update_complex.cache_subnet_group.subnet_ids
+          - subnet_id_c not in update_complex.cache_subnet_group.subnet_ids
+          - subnet_id_d not in update_complex.cache_subnet_group.subnet_ids
+          - update_complex.cache_subnet_group.vpc_id == vpc_id
+          - update_complex.cache_subnet_group.arn.startswith('arn:')
+          - update_complex.cache_subnet_group.arn.endswith(group_name)
+
+    - name: Full Update Subnet Group - idempotency - check_mode
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        description: '{{ description_updated }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+        - '{{ subnet_id_b }}'
+      check_mode: True
+      register: update_complex
+
+    - name: Check result - Full Update Subnet Group - idempotency - check_mode
+      assert:
+        that:
+          - update_complex is successful
+          - update_complex is not changed
+
+    - name: Full Update Subnet Group - idempotency
+      elasticache_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        description: '{{ description_updated }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+        - '{{ subnet_id_b }}'
+      register: update_complex
+
+    - name: Check result - Full Update Subnet Group - idempotency
+      assert:
+        that:
+          - update_complex is successful
+          - update_complex is not changed
+          - '"cache_subnet_group" in update_complex'
+          - '"arn" in update_complex.cache_subnet_group'
+          - '"description" in update_complex.cache_subnet_group'
+          - '"name" in update_complex.cache_subnet_group'
+          - '"subnet_ids" in update_complex.cache_subnet_group'
+          - '"vpc_id" in update_complex.cache_subnet_group'
+          - update_complex.cache_subnet_group.description == description_updated
+          - update_complex.cache_subnet_group.name == group_name
+          - subnet_id_a in update_complex.cache_subnet_group.subnet_ids
+          - subnet_id_b in update_complex.cache_subnet_group.subnet_ids
+          - subnet_id_c not in update_complex.cache_subnet_group.subnet_ids
+          - subnet_id_d not in update_complex.cache_subnet_group.subnet_ids
+          - update_complex.cache_subnet_group.vpc_id == vpc_id
+          - update_complex.cache_subnet_group.arn.startswith('arn:')
+          - update_complex.cache_subnet_group.arn.endswith(group_name)
+
+    # ============================================================
+
+    - name: Delete Subnet Group
+      elasticache_subnet_group:
+        state: absent
+        name: '{{ group_name }}'
+      register: delete_group
+
+    - name: Check result - Delete Subnet Group
+      assert:
+        that:
+          - delete_group is changed
+
   always:
 
     ################################################