From c21eac881a3513995b11eb4200c7b21900d5579e Mon Sep 17 00:00:00 2001
From: Mark Chappell <mchappel@redhat.com>
Date: Fri, 17 Sep 2021 23:56:02 +0200
Subject: [PATCH 1/5] Migrate redshift_subnet_group to boto3

---
 plugins/modules/redshift_subnet_group.py      | 242 +++++++++++-------
 .../redshift_subnet_group/tasks/main.yml      |  29 ++-
 2 files changed, 166 insertions(+), 105 deletions(-)

diff --git a/plugins/modules/redshift_subnet_group.py b/plugins/modules/redshift_subnet_group.py
index 05397286041..dae91d86750 100644
--- a/plugins/modules/redshift_subnet_group.py
+++ b/plugins/modules/redshift_subnet_group.py
@@ -9,8 +9,6 @@
 
 DOCUMENTATION = r'''
 ---
-author:
-  - "Jens Carl (@j-carl), Hothead Games Inc."
 module: redshift_subnet_group
 version_added: 1.0.0
 short_description: manage Redshift cluster subnet groups
@@ -20,33 +18,31 @@
   state:
     description:
       - Specifies whether the subnet should be present or absent.
-    required: true
+    default: 'present'
     choices: ['present', 'absent' ]
     type: str
-  group_name:
+  name:
     description:
       - Cluster subnet group name.
     required: true
-    aliases: ['name']
+    aliases: ['group_name']
     type: str
-  group_description:
+  description:
     description:
-      - Database subnet group description.
-      - Required when I(state=present).
-    aliases: ['description']
+      - Cluster subnet group description.
+    aliases: ['group_description']
     type: str
-  group_subnets:
+  subnets:
     description:
       - List of subnet IDs that make up the cluster subnet group.
-      - Required when I(state=present).
-    aliases: ['subnets']
+    aliases: ['group_subnets']
     type: list
     elements: str
 extends_documentation_fragment:
 - amazon.aws.aws
 - amazon.aws.ec2
-requirements:
-- boto >= 2.49.0
+author:
+  - "Jens Carl (@j-carl), Hothead Games Inc."
 '''
 
 EXAMPLES = r'''
@@ -66,10 +62,10 @@
 '''
 
 RETURN = r'''
-group:
-    description: dictionary containing all Redshift subnet group information
+cluster_subnet_group:
+    description: dictionary containing Redshift subnet group information
     returned: success
-    type: complex
+    type: dict
     contains:
         name:
             description: name of the Redshift subnet group
@@ -84,95 +80,159 @@
 '''
 
 try:
-    import boto
-    import boto.redshift
+    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_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 connect_to_aws
-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 main():
-    argument_spec = dict(
-        state=dict(required=True, choices=['present', 'absent']),
-        group_name=dict(required=True, aliases=['name']),
-        group_description=dict(required=False, aliases=['description']),
-        group_subnets=dict(required=False, aliases=['subnets'], type='list', elements='str'),
-    )
-    module = AnsibleAWSModule(argument_spec=argument_spec, check_boto3=False)
+def get_subnet_group(name):
+    try:
+        groups = client.describe_cluster_subnet_groups(
+            aws_retry=True,
+            ClusterSubnetGroupName=name,
+        )['ClusterSubnetGroups']
+    except is_boto3_error_code('ClusterSubnetGroupNotFoundFault'):
+        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 HAS_BOTO:
-        module.fail_json(msg='boto v2.9.0+ required for this module')
+    if not groups:
+        return None
 
-    state = module.params.get('state')
-    group_name = module.params.get('group_name')
-    group_description = module.params.get('group_description')
-    group_subnets = module.params.get('group_subnets')
+    if len(groups) > 1:
+        module.fail_aws(
+            msg="Found multiple matches for subnet group",
+            cluster_subnet_groups=camel_dict_to_snake_dict(groups),
+        )
 
-    if state == 'present':
-        for required in ('group_name', 'group_description', 'group_subnets'):
-            if not module.params.get(required):
-                module.fail_json(msg=str("parameter %s required for state='present'" % required))
-    else:
-        for not_allowed in ('group_description', 'group_subnets'):
-            if module.params.get(not_allowed):
-                module.fail_json(msg=str("parameter %s not allowed for state='absent'" % not_allowed))
+    subnet_group = camel_dict_to_snake_dict(groups[0])
+
+    subnet_group['name'] = subnet_group['cluster_subnet_group_name']
 
-    region, ec2_url, aws_connect_params = get_aws_connection_info(module)
-    if not region:
-        module.fail_json(msg=str("Region must be specified as a parameter, in EC2_REGION or AWS_REGION environment variables or in boto configuration file"))
+    subnet_ids = list(s['subnet_identifier'] for s in subnet_group['subnets'])
+    subnet_group['subnet_ids'] = subnet_ids
 
-    # Connect to the Redshift endpoint.
+    return subnet_group
+
+
+def create_subnet_group(name, description, subnets):
     try:
-        conn = connect_to_aws(boto.redshift, region, **aws_connect_params)
-    except boto.exception.JSONResponseError as e:
-        module.fail_json(msg=str(e))
+        if not description:
+            description = name
+        if not subnets:
+            subnets = []
+        client.create_cluster_subnet_group(
+            aws_retry=True,
+            ClusterSubnetGroupName=name,
+            Description=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['Description'] = 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
+
+    # Description is optional, SubnetIds is not
+    if 'SubnetIds' not in update_params:
+        update_params['SubnetIds'] = subnet_group['subnet_ids']
 
     try:
-        changed = False
-        exists = False
-        group = None
-
-        try:
-            matching_groups = conn.describe_cluster_subnet_groups(group_name, max_records=100)
-            exists = len(matching_groups) > 0
-        except boto.exception.JSONResponseError as e:
-            if e.body['Error']['Code'] != 'ClusterSubnetGroupNotFoundFault':
-                # if e.code != 'ClusterSubnetGroupNotFoundFault':
-                module.fail_json(msg=str(e))
-
-        if state == 'absent':
-            if exists:
-                conn.delete_cluster_subnet_group(group_name)
-                changed = True
+        client.modify_cluster_subnet_group(
+            aws_retry=True,
+            ClusterSubnetGroupName=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_cluster_subnet_group(
+            aws_retry=True,
+            ClusterSubnetGroupName=name,
+        )
+        return True
+    except is_boto3_error_code('ClusterSubnetGroupNotFoundFault'):
+        # 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(default='present', choices=['present', 'absent']),
+        name=dict(required=True, aliases=['group_name']),
+        description=dict(required=False, aliases=['group_description']),
+        subnets=dict(required=False, aliases=['group_subnets'], type='list', elements='str'),
+    )
+
+    global module
+    global client
+
+    module = AnsibleAWSModule(
+        argument_spec=argument_spec,
+    )
+
+    state = module.params.get('state')
+    name = module.params.get('group_name')
+    description = module.params.get('group_description')
+    subnets = module.params.get('group_subnets')
+
+    client = module.client('redshift', retry_decorator=AWSRetry.jittered_backoff())
+
+    subnet_group = get_subnet_group(name)
+    changed = False
+
+    if state == 'present':
+        if not subnet_group:
+            result = create_subnet_group(name, description, subnets)
+            changed |= result
         else:
-            if not exists:
-                new_group = conn.create_cluster_subnet_group(group_name, group_description, group_subnets)
-                group = {
-                    'name': new_group['CreateClusterSubnetGroupResponse']['CreateClusterSubnetGroupResult']
-                    ['ClusterSubnetGroup']['ClusterSubnetGroupName'],
-                    'vpc_id': new_group['CreateClusterSubnetGroupResponse']['CreateClusterSubnetGroupResult']
-                    ['ClusterSubnetGroup']['VpcId'],
-                }
-            else:
-                changed_group = conn.modify_cluster_subnet_group(group_name, group_subnets, description=group_description)
-                group = {
-                    'name': changed_group['ModifyClusterSubnetGroupResponse']['ModifyClusterSubnetGroupResult']
-                    ['ClusterSubnetGroup']['ClusterSubnetGroupName'],
-                    'vpc_id': changed_group['ModifyClusterSubnetGroupResponse']['ModifyClusterSubnetGroupResult']
-                    ['ClusterSubnetGroup']['VpcId'],
-                }
-
-            changed = True
-
-    except boto.exception.JSONResponseError as e:
-        module.fail_json(msg=str(e))
-
-    module.exit_json(changed=changed, group=group)
+            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
+
+    compat_results = dict()
+    if subnet_group:
+        compat_results['group'] = dict(
+            name=subnet_group['name'],
+            vpc_id=subnet_group['vpc_id'],
+        )
+
+    module.exit_json(
+        changed=changed,
+        cluster_subnet_group=subnet_group,
+        **compat_results,
+    )
 
 
 if __name__ == '__main__':
diff --git a/tests/integration/targets/redshift_subnet_group/tasks/main.yml b/tests/integration/targets/redshift_subnet_group/tasks/main.yml
index bdef77d8ac5..23758403511 100644
--- a/tests/integration/targets/redshift_subnet_group/tasks/main.yml
+++ b/tests/integration/targets/redshift_subnet_group/tasks/main.yml
@@ -93,8 +93,7 @@
       assert:
         that:
           - create_group is successful
-          # XXX Not idempotent
-          #- create_group is not changed
+          - create_group is not changed
           - '"group" in create_group'
           - '"name" in create_group.group'
           - '"vpc_id" in create_group.group'
@@ -108,9 +107,10 @@
         state: present
         group_name: '{{ group_name }}'
         group_description: '{{ description_updated }}'
-        group_subnets:
-        - '{{ subnet_id_a }}'
-        - '{{ subnet_id_b }}'
+        ## No longer mandatory
+        # group_subnets:
+        # - '{{ subnet_id_a }}'
+        # - '{{ subnet_id_b }}'
       register: update_description
 
     - name: Check result - Update Subnet Group Description
@@ -129,17 +129,17 @@
         state: present
         group_name: '{{ group_name }}'
         group_description: '{{ description_updated }}'
-        group_subnets:
-        - '{{ subnet_id_a }}'
-        - '{{ subnet_id_b }}'
+        ## No longer mandatory
+        # group_subnets:
+        # - '{{ subnet_id_a }}'
+        # - '{{ subnet_id_b }}'
       register: update_description
 
     - name: Check result - Update Subnet Group Description - idempotency
       assert:
         that:
           - update_description is successful
-          # XXX Not idempotent
-          #- update_description is not changed
+          - update_description is not changed
           - '"group" in update_description'
           - '"name" in update_description.group'
           - '"vpc_id" in update_description.group'
@@ -152,7 +152,8 @@
       redshift_subnet_group:
         state: present
         group_name: '{{ group_name }}'
-        group_description: '{{ description_updated }}'
+        ## No longer mandatory
+        # group_description: '{{ description_updated }}'
         group_subnets:
         - '{{ subnet_id_c }}'
         - '{{ subnet_id_d }}'
@@ -173,7 +174,8 @@
       redshift_subnet_group:
         state: present
         group_name: '{{ group_name }}'
-        group_description: '{{ description_updated }}'
+        ## No longer mandatory
+        # group_description: '{{ description_updated }}'
         group_subnets:
         - '{{ subnet_id_c }}'
         - '{{ subnet_id_d }}'
@@ -183,8 +185,7 @@
       assert:
         that:
           - update_subnets is successful
-          # XXX Not idempotent
-          #- update_subnets is not changed
+          - update_subnets is not changed
           - '"group" in update_subnets'
           - '"name" in update_subnets.group'
           - '"vpc_id" in update_subnets.group'

From dc4cd6e116b55baf9a8ae0838dfb56daee4d4a79 Mon Sep 17 00:00:00 2001
From: Mark Chappell <mchappel@redhat.com>
Date: Sat, 18 Sep 2021 09:12:11 +0200
Subject: [PATCH 2/5] Add additional tests for minimal/partial parameters.

---
 plugins/modules/redshift_subnet_group.py      |  15 ++-
 .../redshift_subnet_group/tasks/main.yml      | 120 ++++++++++++++++++
 2 files changed, 129 insertions(+), 6 deletions(-)

diff --git a/plugins/modules/redshift_subnet_group.py b/plugins/modules/redshift_subnet_group.py
index dae91d86750..c67a46e4c00 100644
--- a/plugins/modules/redshift_subnet_group.py
+++ b/plugins/modules/redshift_subnet_group.py
@@ -17,7 +17,7 @@
 options:
   state:
     description:
-      - Specifies whether the subnet should be present or absent.
+      - Specifies whether the subnet group should be present or absent.
     default: 'present'
     choices: ['present', 'absent' ]
     type: str
@@ -35,6 +35,7 @@
   subnets:
     description:
       - List of subnet IDs that make up the cluster subnet group.
+      - At least one subnet must be provided when creating a cluster subnet group.
     aliases: ['group_subnets']
     type: list
     elements: str
@@ -122,11 +123,13 @@ 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')
+
     try:
         if not description:
             description = name
-        if not subnets:
-            subnets = []
         client.create_cluster_subnet_group(
             aws_retry=True,
             ClusterSubnetGroupName=name,
@@ -198,9 +201,9 @@ def main():
     )
 
     state = module.params.get('state')
-    name = module.params.get('group_name')
-    description = module.params.get('group_description')
-    subnets = module.params.get('group_subnets')
+    name = module.params.get('name')
+    description = module.params.get('description')
+    subnets = module.params.get('subnets')
 
     client = module.client('redshift', retry_decorator=AWSRetry.jittered_backoff())
 
diff --git a/tests/integration/targets/redshift_subnet_group/tasks/main.yml b/tests/integration/targets/redshift_subnet_group/tasks/main.yml
index 23758403511..b1d269235e6 100644
--- a/tests/integration/targets/redshift_subnet_group/tasks/main.yml
+++ b/tests/integration/targets/redshift_subnet_group/tasks/main.yml
@@ -14,6 +14,28 @@
       security_token: '{{ security_token | default(omit) }}'
       region: '{{ aws_region }}'
   block:
+
+    # ============================================================
+
+    - name: Create Subnet Group with no subnets
+      redshift_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
+          - '"CreateClusterSubnetGroup" 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
@@ -56,6 +78,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
@@ -216,6 +239,103 @@
         that:
           - delete_group is not changed
 
+    # ============================================================
+
+    - name: Create minimal Subnet Group
+      redshift_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
+          - '"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
+
+    - name: Create minimal Subnet Group - idempotency
+      redshift_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
+          - '"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
+
+    # ============================================================
+
+    - name: Full Update Subnet Group
+      redshift_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
+          - '"group" in update_complex'
+          - '"name" in update_complex.group'
+          - '"vpc_id" in update_complex.group'
+          - update_complex.group.name == group_name
+          - update_complex.group.vpc_id == vpc_id
+
+    - name: Full Update Subnet Group - idempotency
+      redshift_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
+          - '"group" in update_complex'
+          - '"name" in update_complex.group'
+          - '"vpc_id" in update_complex.group'
+          - update_complex.group.name == group_name
+          - update_complex.group.vpc_id == vpc_id
+
+    # ============================================================
+
+    - name: Delete Subnet Group
+      redshift_subnet_group:
+        state: absent
+        name: '{{ group_name }}'
+      register: delete_group
+
+    - name: Check result - Delete Subnet Group
+      assert:
+        that:
+          - delete_group is changed
+
   always:
 
     ################################################

From aac8190f737173247f871c57196dea58f65a1f9e Mon Sep 17 00:00:00 2001
From: Mark Chappell <mchappel@redhat.com>
Date: Sat, 18 Sep 2021 09:59:28 +0200
Subject: [PATCH 3/5] Add additional tests and documentation around new return
 values

---
 plugins/modules/redshift_subnet_group.py      |  26 ++++-
 .../redshift_subnet_group/tasks/main.yml      | 110 ++++++++++++++++++
 2 files changed, 131 insertions(+), 5 deletions(-)

diff --git a/plugins/modules/redshift_subnet_group.py b/plugins/modules/redshift_subnet_group.py
index c67a46e4c00..a609dd47a2c 100644
--- a/plugins/modules/redshift_subnet_group.py
+++ b/plugins/modules/redshift_subnet_group.py
@@ -64,20 +64,33 @@
 
 RETURN = r'''
 cluster_subnet_group:
-    description: dictionary containing Redshift subnet group information
+    description: A dictionary containing information about the Redshift subnet group.
     returned: success
     type: dict
     contains:
         name:
-            description: name of the Redshift subnet group
-            returned: success
+            description: Name of the Redshift subnet group.
+            returned: when the cache subnet group exists
             type: str
             sample: "redshift_subnet_group_name"
         vpc_id:
-            description: Id of the VPC where the subnet is located
-            returned: success
+            description: Id of the VPC where the subnet is located.
+            returned: when the cache subnet group exists
             type: str
             sample: "vpc-aabb1122"
+        description:
+            description: The description of the cache subnet group.
+            returned: when the cache subnet group exists
+            type: str
+            sample: Redshift subnet
+        subnet_ids:
+            description: The IDs of the subnets beloging to the Redshift subnet group.
+            returned: when the cache subnet group exists
+            type: list
+            elements: str
+            sample:
+              - subnet-aaaaaaaa
+              - subnet-bbbbbbbb
 '''
 
 try:
@@ -90,6 +103,7 @@
 from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule
 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
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict
 
 
 def get_subnet_group(name):
@@ -112,8 +126,10 @@ def get_subnet_group(name):
             cluster_subnet_groups=camel_dict_to_snake_dict(groups),
         )
 
+    tags = boto3_tag_list_to_ansible_dict(groups[0]['Tags'])
     subnet_group = camel_dict_to_snake_dict(groups[0])
 
+    subnet_group['tags'] = tags
     subnet_group['name'] = subnet_group['cluster_subnet_group_name']
 
     subnet_ids = list(s['subnet_identifier'] for s in subnet_group['subnets'])
diff --git a/tests/integration/targets/redshift_subnet_group/tasks/main.yml b/tests/integration/targets/redshift_subnet_group/tasks/main.yml
index b1d269235e6..c8f8552d758 100644
--- a/tests/integration/targets/redshift_subnet_group/tasks/main.yml
+++ b/tests/integration/targets/redshift_subnet_group/tasks/main.yml
@@ -101,6 +101,17 @@
           - '"vpc_id" in create_group.group'
           - create_group.group.name == group_name
           - create_group.group.vpc_id == vpc_id
+          - '"cluster_subnet_group" in create_group'
+          - '"description" in create_group.cluster_subnet_group'
+          - '"subnet_ids" in create_group.cluster_subnet_group'
+          - '"vpc_id" in create_group.cluster_subnet_group'
+          - create_group.cluster_subnet_group.name == group_name
+          - create_group.cluster_subnet_group.description == description_default
+          - subnet_id_a in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_b in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_c not in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_d not in create_group.cluster_subnet_group.subnet_ids
+          - create_group.cluster_subnet_group.vpc_id == vpc_id
 
     - name: Create Subnet Group - idempotency
       redshift_subnet_group:
@@ -122,6 +133,17 @@
           - '"vpc_id" in create_group.group'
           - create_group.group.name == group_name
           - create_group.group.vpc_id == vpc_id
+          - '"cluster_subnet_group" in create_group'
+          - '"description" in create_group.cluster_subnet_group'
+          - '"subnet_ids" in create_group.cluster_subnet_group'
+          - '"vpc_id" in create_group.cluster_subnet_group'
+          - create_group.cluster_subnet_group.name == group_name
+          - create_group.cluster_subnet_group.description == description_default
+          - subnet_id_a in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_b in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_c not in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_d not in create_group.cluster_subnet_group.subnet_ids
+          - create_group.cluster_subnet_group.vpc_id == vpc_id
 
     # ============================================================
 
@@ -146,6 +168,17 @@
           - '"vpc_id" in update_description.group'
           - update_description.group.name == group_name
           - update_description.group.vpc_id == vpc_id
+          - '"cluster_subnet_group" in update_description'
+          - '"description" in update_description.cluster_subnet_group'
+          - '"subnet_ids" in update_description.cluster_subnet_group'
+          - '"vpc_id" in update_description.cluster_subnet_group'
+          - update_description.cluster_subnet_group.name == group_name
+          - update_description.cluster_subnet_group.description == description_updated
+          - subnet_id_a in update_description.cluster_subnet_group.subnet_ids
+          - subnet_id_b in update_description.cluster_subnet_group.subnet_ids
+          - subnet_id_c not in update_description.cluster_subnet_group.subnet_ids
+          - subnet_id_d not in update_description.cluster_subnet_group.subnet_ids
+          - update_description.cluster_subnet_group.vpc_id == vpc_id
 
     - name: Update Subnet Group Description - idempotency
       redshift_subnet_group:
@@ -168,6 +201,17 @@
           - '"vpc_id" in update_description.group'
           - update_description.group.name == group_name
           - update_description.group.vpc_id == vpc_id
+          - '"cluster_subnet_group" in update_description'
+          - '"description" in update_description.cluster_subnet_group'
+          - '"subnet_ids" in update_description.cluster_subnet_group'
+          - '"vpc_id" in update_description.cluster_subnet_group'
+          - update_description.cluster_subnet_group.name == group_name
+          - update_description.cluster_subnet_group.description == description_updated
+          - subnet_id_a in update_description.cluster_subnet_group.subnet_ids
+          - subnet_id_b in update_description.cluster_subnet_group.subnet_ids
+          - subnet_id_c not in update_description.cluster_subnet_group.subnet_ids
+          - subnet_id_d not in update_description.cluster_subnet_group.subnet_ids
+          - update_description.cluster_subnet_group.vpc_id == vpc_id
 
     # ============================================================
 
@@ -192,6 +236,17 @@
           - '"vpc_id" in update_subnets.group'
           - update_subnets.group.name == group_name
           - update_subnets.group.vpc_id == vpc_id
+          - '"cluster_subnet_group" in update_subnets'
+          - '"description" in update_subnets.cluster_subnet_group'
+          - '"subnet_ids" in update_subnets.cluster_subnet_group'
+          - '"vpc_id" in update_subnets.cluster_subnet_group'
+          - update_subnets.cluster_subnet_group.name == group_name
+          - update_subnets.cluster_subnet_group.description == description_updated
+          - subnet_id_a not in update_subnets.cluster_subnet_group.subnet_ids
+          - subnet_id_b not in update_subnets.cluster_subnet_group.subnet_ids
+          - subnet_id_c in update_subnets.cluster_subnet_group.subnet_ids
+          - subnet_id_d in update_subnets.cluster_subnet_group.subnet_ids
+          - update_subnets.cluster_subnet_group.vpc_id == vpc_id
 
     - name: Update Subnet Group subnets - idempotency
       redshift_subnet_group:
@@ -214,6 +269,17 @@
           - '"vpc_id" in update_subnets.group'
           - update_subnets.group.name == group_name
           - update_subnets.group.vpc_id == vpc_id
+          - '"cluster_subnet_group" in update_subnets'
+          - '"description" in update_subnets.cluster_subnet_group'
+          - '"subnet_ids" in update_subnets.cluster_subnet_group'
+          - '"vpc_id" in update_subnets.cluster_subnet_group'
+          - update_subnets.cluster_subnet_group.name == group_name
+          - update_subnets.cluster_subnet_group.description == description_updated
+          - subnet_id_a not in update_subnets.cluster_subnet_group.subnet_ids
+          - subnet_id_b not in update_subnets.cluster_subnet_group.subnet_ids
+          - subnet_id_c in update_subnets.cluster_subnet_group.subnet_ids
+          - subnet_id_d in update_subnets.cluster_subnet_group.subnet_ids
+          - update_subnets.cluster_subnet_group.vpc_id == vpc_id
 
     # ============================================================
 
@@ -259,6 +325,17 @@
           - '"vpc_id" in create_group.group'
           - create_group.group.name == group_name
           - create_group.group.vpc_id == vpc_id
+          - '"cluster_subnet_group" in create_group'
+          - '"description" in create_group.cluster_subnet_group'
+          - '"subnet_ids" in create_group.cluster_subnet_group'
+          - '"vpc_id" in create_group.cluster_subnet_group'
+          - create_group.cluster_subnet_group.name == group_name
+          - create_group.cluster_subnet_group.description == group_name
+          - subnet_id_a in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_b not in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_c not in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_d not in create_group.cluster_subnet_group.subnet_ids
+          - create_group.cluster_subnet_group.vpc_id == vpc_id
 
     - name: Create minimal Subnet Group - idempotency
       redshift_subnet_group:
@@ -278,6 +355,17 @@
           - '"vpc_id" in create_group.group'
           - create_group.group.name == group_name
           - create_group.group.vpc_id == vpc_id
+          - '"cluster_subnet_group" in create_group'
+          - '"description" in create_group.cluster_subnet_group'
+          - '"subnet_ids" in create_group.cluster_subnet_group'
+          - '"vpc_id" in create_group.cluster_subnet_group'
+          - create_group.cluster_subnet_group.name == group_name
+          - create_group.cluster_subnet_group.description == group_name
+          - subnet_id_a in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_b not in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_c not in create_group.cluster_subnet_group.subnet_ids
+          - subnet_id_d not in create_group.cluster_subnet_group.subnet_ids
+          - create_group.cluster_subnet_group.vpc_id == vpc_id
 
     # ============================================================
 
@@ -301,6 +389,17 @@
           - '"vpc_id" in update_complex.group'
           - update_complex.group.name == group_name
           - update_complex.group.vpc_id == vpc_id
+          - '"cluster_subnet_group" in update_complex'
+          - '"description" in update_complex.cluster_subnet_group'
+          - '"subnet_ids" in update_complex.cluster_subnet_group'
+          - '"vpc_id" in update_complex.cluster_subnet_group'
+          - update_complex.cluster_subnet_group.name == group_name
+          - update_complex.cluster_subnet_group.description == description_updated
+          - subnet_id_a in update_complex.cluster_subnet_group.subnet_ids
+          - subnet_id_b in update_complex.cluster_subnet_group.subnet_ids
+          - subnet_id_c not in update_complex.cluster_subnet_group.subnet_ids
+          - subnet_id_d not in update_complex.cluster_subnet_group.subnet_ids
+          - update_complex.cluster_subnet_group.vpc_id == vpc_id
 
     - name: Full Update Subnet Group - idempotency
       redshift_subnet_group:
@@ -322,6 +421,17 @@
           - '"vpc_id" in update_complex.group'
           - update_complex.group.name == group_name
           - update_complex.group.vpc_id == vpc_id
+          - '"cluster_subnet_group" in update_complex'
+          - '"description" in update_complex.cluster_subnet_group'
+          - '"subnet_ids" in update_complex.cluster_subnet_group'
+          - '"vpc_id" in update_complex.cluster_subnet_group'
+          - update_complex.cluster_subnet_group.name == group_name
+          - update_complex.cluster_subnet_group.description == description_updated
+          - subnet_id_a in update_complex.cluster_subnet_group.subnet_ids
+          - subnet_id_b in update_complex.cluster_subnet_group.subnet_ids
+          - subnet_id_c not in update_complex.cluster_subnet_group.subnet_ids
+          - subnet_id_d not in update_complex.cluster_subnet_group.subnet_ids
+          - update_complex.cluster_subnet_group.vpc_id == vpc_id
 
     # ============================================================
 

From 451bef72306b8c62212dc44e553d7bd43d9fbf63 Mon Sep 17 00:00:00 2001
From: Mark Chappell <mchappel@redhat.com>
Date: Sat, 18 Sep 2021 10:13:55 +0200
Subject: [PATCH 4/5] Add documentation and support for check_mode

---
 plugins/modules/redshift_subnet_group.py      |  14 ++
 .../redshift_subnet_group/tasks/main.yml      | 214 ++++++++++++++++++
 2 files changed, 228 insertions(+)

diff --git a/plugins/modules/redshift_subnet_group.py b/plugins/modules/redshift_subnet_group.py
index a609dd47a2c..89e8bfa8042 100644
--- a/plugins/modules/redshift_subnet_group.py
+++ b/plugins/modules/redshift_subnet_group.py
@@ -126,7 +126,10 @@ def get_subnet_group(name):
             cluster_subnet_groups=camel_dict_to_snake_dict(groups),
         )
 
+    # No support for managing tags yet, but make sure that we don't need to
+    # change the return value structure after it's been available in a release.
     tags = boto3_tag_list_to_ansible_dict(groups[0]['Tags'])
+
     subnet_group = camel_dict_to_snake_dict(groups[0])
 
     subnet_group['tags'] = tags
@@ -143,6 +146,9 @@ 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
@@ -170,6 +176,9 @@ def update_subnet_group(subnet_group, name, description, subnets):
     if not update_params:
         return False
 
+    if module.check_mode:
+        return True
+
     # Description is optional, SubnetIds is not
     if 'SubnetIds' not in update_params:
         update_params['SubnetIds'] = subnet_group['subnet_ids']
@@ -187,6 +196,10 @@ def update_subnet_group(subnet_group, name, description, subnets):
 
 
 def delete_subnet_group(name):
+
+    if module.check_mode:
+        return True
+
     try:
         client.delete_cluster_subnet_group(
             aws_retry=True,
@@ -214,6 +227,7 @@ def main():
 
     module = AnsibleAWSModule(
         argument_spec=argument_spec,
+        supports_check_mode=True,
     )
 
     state = module.params.get('state')
diff --git a/tests/integration/targets/redshift_subnet_group/tasks/main.yml b/tests/integration/targets/redshift_subnet_group/tasks/main.yml
index c8f8552d758..e15ee9b9313 100644
--- a/tests/integration/targets/redshift_subnet_group/tasks/main.yml
+++ b/tests/integration/targets/redshift_subnet_group/tasks/main.yml
@@ -17,6 +17,26 @@
 
     # ============================================================
 
+    - name: Create Subnet Group with no subnets - check_mode
+      redshift_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+      register: create_group
+      ignore_errors: True
+      check_mode: 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
+          - '"CreateClusterSubnetGroup" 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
       redshift_subnet_group:
         state: present
@@ -81,6 +101,23 @@
 
     # ============================================================
 
+    - name: Create Subnet Group - check_mode
+      redshift_subnet_group:
+        state: present
+        group_name: '{{ group_name }}'
+        group_description: '{{ description_default }}'
+        group_subnets:
+        - '{{ subnet_id_a }}'
+        - '{{ subnet_id_b }}'
+      register: create_group
+      check_mode: True
+
+    - name: Check result - Create Subnet Group - check_mode
+      assert:
+        that:
+          - create_group is successful
+          - create_group is changed
+
     - name: Create Subnet Group
       redshift_subnet_group:
         state: present
@@ -113,6 +150,23 @@
           - subnet_id_d not in create_group.cluster_subnet_group.subnet_ids
           - create_group.cluster_subnet_group.vpc_id == vpc_id
 
+    - name: Create Subnet Group - idempotency - check_mode
+      redshift_subnet_group:
+        state: present
+        group_name: '{{ group_name }}'
+        group_description: '{{ description_default }}'
+        group_subnets:
+        - '{{ subnet_id_a }}'
+        - '{{ subnet_id_b }}'
+      register: create_group
+      check_mode: True
+
+    - 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
       redshift_subnet_group:
         state: present
@@ -147,6 +201,24 @@
 
     # ============================================================
 
+    - name: Update Subnet Group Description - check_mode
+      redshift_subnet_group:
+        state: present
+        group_name: '{{ group_name }}'
+        group_description: '{{ description_updated }}'
+        ## No longer mandatory
+        # group_subnets:
+        # - '{{ subnet_id_a }}'
+        # - '{{ subnet_id_b }}'
+      register: update_description
+      check_mode: True
+
+    - name: Check result - Update Subnet Group Description - check_mode
+      assert:
+        that:
+          - update_description is successful
+          - update_description is changed
+
     - name: Update Subnet Group Description
       redshift_subnet_group:
         state: present
@@ -180,6 +252,24 @@
           - subnet_id_d not in update_description.cluster_subnet_group.subnet_ids
           - update_description.cluster_subnet_group.vpc_id == vpc_id
 
+    - name: Update Subnet Group Description - idempotency - check_mode
+      redshift_subnet_group:
+        state: present
+        group_name: '{{ group_name }}'
+        group_description: '{{ description_updated }}'
+        ## No longer mandatory
+        # group_subnets:
+        # - '{{ subnet_id_a }}'
+        # - '{{ subnet_id_b }}'
+      register: update_description
+      check_mode: True
+
+    - 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
       redshift_subnet_group:
         state: present
@@ -215,6 +305,24 @@
 
     # ============================================================
 
+    - name: Update Subnet Group subnets - check_mode
+      redshift_subnet_group:
+        state: present
+        group_name: '{{ group_name }}'
+        ## No longer mandatory
+        # group_description: '{{ description_updated }}'
+        group_subnets:
+        - '{{ subnet_id_c }}'
+        - '{{ subnet_id_d }}'
+      register: update_subnets
+      check_mode: True
+
+    - name: Check result - Update Subnet Group subnets - check_mode
+      assert:
+        that:
+          - update_subnets is successful
+          - update_subnets is changed
+
     - name: Update Subnet Group subnets
       redshift_subnet_group:
         state: present
@@ -248,6 +356,24 @@
           - subnet_id_d in update_subnets.cluster_subnet_group.subnet_ids
           - update_subnets.cluster_subnet_group.vpc_id == vpc_id
 
+    - name: Update Subnet Group subnets - idempotency - check_mode
+      redshift_subnet_group:
+        state: present
+        group_name: '{{ group_name }}'
+        ## No longer mandatory
+        # group_description: '{{ description_updated }}'
+        group_subnets:
+        - '{{ subnet_id_c }}'
+        - '{{ subnet_id_d }}'
+      register: update_subnets
+      check_mode: True
+
+    - 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
       redshift_subnet_group:
         state: present
@@ -283,6 +409,18 @@
 
     # ============================================================
 
+    - name: Delete Subnet Group - check_mode
+      redshift_subnet_group:
+        state: absent
+        group_name: '{{ group_name }}'
+      register: delete_group
+      check_mode: True
+
+    - name: Check result - Delete Subnet Group - check_mode
+      assert:
+        that:
+          - delete_group is changed
+
     - name: Delete Subnet Group
       redshift_subnet_group:
         state: absent
@@ -294,6 +432,18 @@
         that:
           - delete_group is changed
 
+    - name: Delete Subnet Group - idempotency - check_mode
+      redshift_subnet_group:
+        state: absent
+        group_name: '{{ group_name }}'
+      register: delete_group
+      check_mode: True
+
+    - name: Check result - Delete Subnet Group - idempotency - check_mode
+      assert:
+        that:
+          - delete_group is not changed
+
     - name: Delete Subnet Group - idempotency
       redshift_subnet_group:
         state: absent
@@ -307,6 +457,21 @@
 
     # ============================================================
 
+    - name: Create minimal Subnet Group - check_mode
+      redshift_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+      register: create_group
+      check_mode: True
+
+    - name: Check result - Create minimal Subnet Group - check_mode
+      assert:
+        that:
+          - create_group is successful
+          - create_group is changed
+
     - name: Create minimal Subnet Group
       redshift_subnet_group:
         state: present
@@ -337,6 +502,21 @@
           - subnet_id_d not in create_group.cluster_subnet_group.subnet_ids
           - create_group.cluster_subnet_group.vpc_id == vpc_id
 
+    - name: Create minimal Subnet Group - idempotency - check_mode
+      redshift_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+      register: create_group
+      check_mode: True
+
+    - 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
       redshift_subnet_group:
         state: present
@@ -369,6 +549,23 @@
 
     # ============================================================
 
+    - name: Full Update Subnet Group - check_mode
+      redshift_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        description: '{{ description_updated }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+        - '{{ subnet_id_b }}'
+      register: update_complex
+      check_mode: True
+
+    - name: Check result - Full Update Subnet Group - check_mode
+      assert:
+        that:
+          - update_complex is successful
+          - update_complex is changed
+
     - name: Full Update Subnet Group
       redshift_subnet_group:
         state: present
@@ -401,6 +598,23 @@
           - subnet_id_d not in update_complex.cluster_subnet_group.subnet_ids
           - update_complex.cluster_subnet_group.vpc_id == vpc_id
 
+    - name: Full Update Subnet Group - idempotency - check_mode
+      redshift_subnet_group:
+        state: present
+        name: '{{ group_name }}'
+        description: '{{ description_updated }}'
+        subnets:
+        - '{{ subnet_id_a }}'
+        - '{{ subnet_id_b }}'
+      register: update_complex
+      check_mode: True
+
+    - 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
       redshift_subnet_group:
         state: present

From 861c9665d79b44137542dfea9cdf926684f50252 Mon Sep 17 00:00:00 2001
From: Mark Chappell <mchappel@redhat.com>
Date: Thu, 23 Sep 2021 09:10:45 +0200
Subject: [PATCH 5/5] changelog

---
 changelogs/fragments/724-redshift_subnet_group-boto3.yml | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 changelogs/fragments/724-redshift_subnet_group-boto3.yml

diff --git a/changelogs/fragments/724-redshift_subnet_group-boto3.yml b/changelogs/fragments/724-redshift_subnet_group-boto3.yml
new file mode 100644
index 00000000000..d760eb4c2b8
--- /dev/null
+++ b/changelogs/fragments/724-redshift_subnet_group-boto3.yml
@@ -0,0 +1,7 @@
+minor_changes:
+- redshift_subnet_group - the module has been migrated to the boto3 AWS SDK (https://github.com/ansible-collections/community.aws/pull/724).
+- redshift_subnet_group - added support for check_mode (https://github.com/ansible-collections/community.aws/pull/724).
+- redshift_subnet_group - the ``group_description`` option has been renamed to ``description`` and is now optional.
+  The old parameter name will continue to work (https://github.com/ansible-collections/community.aws/pull/724).
+- redshift_subnet_group - the ``group_subnets`` option has been renamed to ``subnets`` and is now only required when creating a new group.
+  The old parameter name will continue to work (https://github.com/ansible-collections/community.aws/pull/724).