Refactor rds snapshot modules (ansible-collections#2138)

SUMMARY
Move shared functionality from rds snapshot modules into rds module_utils. Second PR for ansible-collections#2003 / https://issues.redhat.com/browse/ACA-1343.
COMPONENT NAME
rds_cluster_snapshot
rds_instance_snapshot
rds_instance
rds_snapshot_info
module_utils/rds.py
ADDITIONAL INFORMATION
Detailed summary of all the changes:
module_utils/rds.py:

Add describe_db_cluster_snapshots() function
Add get_snapshot() function to retrieve a single db instance or cluster snapshot using internal describe_db_snapshots() and describe_db_cluster_snapshots()
Add format_rds_client_method_parameters() to validate and format parameters for boto3 rds client methods
Update internal collection imports to use full collection path
Add unit tests for new functions

rds_instance module:

Replace get_final_snapshot() function with calls to get_snapshot() from module_utils/rds.py
Replace parameter formatting logic in get_parameters() with call to format_rds_client_method_parameters() from module_utils/rds.py
Remove unit tests for deleted get_final_snapshot() function

rds_instance_snapshot module:

Replace get_snapshot() function with calls to get_snapshot() from module_utils/rds.py
Replace get_parameters() function with call to format_rds_client_method_parameters() from module_utils/rds.py
Remove global variables
Add type hinting to all functions

rds_cluster_snapshot module:

Replace get_parameters() function with call to format_rds_client_method_parameters() from module_utils/rds.py
Remove global variables
Add type hinting to all functions

rds_snapshot_info module:

Refactor internal common_snapshot_info() function to use describe_db_snapshots() and describe_db_cluster_snapshots() functions from module_utils/rds.py
Rename some variables to ensure consistent variable naming
Add type hinting to all functions

Reviewed-by: Alina Buzachis
Reviewed-by: Mark Chappell
Reviewed-by: Helen Bailey <[email protected]>
Reviewed-by: Bikouo Aubin

changelogs/fragments/refactor_rds_snapshot_modules.yml

@@ -0,0 +1,8 @@
+---
+minor_changes:
+  - module_utils/rds.py - Add shared functionality from rds snapshot modules (https://github.com/ansible-collections/amazon.aws/pull/2138).
+  - rds_cluster_snapshot - Refactor shared boto3 client functionality, add type hinting and function docstrings (https://github.com/ansible-collections/amazon.aws/pull/2138).
+  - rds_instance - Remove shared functioanlity added to module_utils/rds.py (https://github.com/ansible-collections/amazon.aws/pull/2138).
+  - rds_instance_info - Refactor shared boto3 client functionality, add type hinting and function docstrings (https://github.com/ansible-collections/amazon.aws/pull/2138).
+  - rds_instance_snapshot - Refactor shared boto3 client functionality, add type hinting and function docstrings (https://github.com/ansible-collections/amazon.aws/pull/2138).
+  - rds_snapshot_info - Refactor shared boto3 client functionality, add type hinting and function docstrings (https://github.com/ansible-collections/amazon.aws/pull/2138).

plugins/module_utils/rds.py

@@ -21,15 +21,16 @@
 from ansible.module_utils._text import to_text
 from ansible.module_utils.common.dict_transformations import snake_dict_to_camel_dict
 
-from .botocore import is_boto3_error_code
-from .core import AnsibleAWSModule
-from .errors import AWSErrorHandler
-from .exceptions import AnsibleAWSError
-from .retries import AWSRetry
-from .tagging import ansible_dict_to_boto3_tag_list
-from .tagging import boto3_tag_list_to_ansible_dict
-from .tagging import compare_aws_tags
-from .waiters import get_waiter
+from ansible_collections.amazon.aws.plugins.module_utils.botocore import get_boto3_client_method_parameters
+from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code
+from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule
+from ansible_collections.amazon.aws.plugins.module_utils.errors import AWSErrorHandler
+from ansible_collections.amazon.aws.plugins.module_utils.exceptions import AnsibleAWSError
+from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry
+from ansible_collections.amazon.aws.plugins.module_utils.tagging import ansible_dict_to_boto3_tag_list
+from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict
+from ansible_collections.amazon.aws.plugins.module_utils.tagging import compare_aws_tags
+from ansible_collections.amazon.aws.plugins.module_utils.waiters import get_waiter
 
 Boto3ClientMethod = namedtuple(
     "Boto3ClientMethod", ["name", "waiter", "operation_description", "resource", "retry_codes"]
@@ -96,7 +97,16 @@ class RDSErrorHandler(AWSErrorHandler):
 
     @classmethod
     def _is_missing(cls):
-        return is_boto3_error_code(["DBInstanceNotFound", "DBSnapshotNotFound", "DBClusterNotFound"])
+        return is_boto3_error_code(
+            ["DBInstanceNotFound", "DBSnapshotNotFound", "DBClusterNotFound", "DBClusterSnapshotNotFoundFault"]
+        )
+
+
+@RDSErrorHandler.list_error_handler("describe db cluster snapshots", [])
+@AWSRetry.jittered_backoff()
+def describe_db_cluster_snapshots(client, **params: Dict) -> List[Dict[str, Any]]:
+    paginator = client.get_paginator("describe_db_cluster_snapshots")
+    return paginator.paginate(**params).build_full_result()["DBClusterSnapshots"]
 
 
 @RDSErrorHandler.list_error_handler("describe db instances", [])
@@ -492,6 +502,40 @@ def wait_for_status(client, module: AnsibleAWSModule, identifier: str, method_na
         wait_for_cluster_snapshot_status(client, module, identifier, waiter_name)
 
 
+def get_snapshot(client, snapshot_identifier: str, snapshot_type: str, convert_tags: bool = True) -> Dict[str, Any]:
+    """
+    Returns instance or cluster snapshot attributes given the snapshot identifier.
+
+        Parameters:
+            client: boto3 rds client
+            snapshot_identifier (str): Unique snapshot identifier
+            snapshot_type (str): Which type of snapshot to get, one of: cluster, instance
+            convert_tags (bool): Whether to convert the snapshot tags from boto3 list of dicts to Ansible dict; defaults to True
+
+        Returns:
+            snapshot (dict): Snapshot attributes. If snapshot with provided id is not found, returns an empty dict
+
+        Raises:
+            ValueError if an invalid snapshot_type is passed
+    """
+    valid_types = ("cluster", "instance")
+    if snapshot_type not in valid_types:
+        raise ValueError(f"Invalid snapshot_type. Expected one of: {valid_types}")
+
+    snapshot = {}
+    if snapshot_type == "cluster":
+        snapshots = describe_db_cluster_snapshots(client, DBClusterSnapshotIdentifier=snapshot_identifier)
+    elif snapshot_type == "instance":
+        snapshots = describe_db_snapshots(client, DBSnapshotIdentifier=snapshot_identifier)
+    if snapshots:
+        snapshot = snapshots[0]
+
+    if snapshot and convert_tags:
+        snapshot["Tags"] = boto3_tag_list_to_ansible_dict(snapshot.pop("TagList"))
+
+    return snapshot
+
+
 def get_tags(client, module: AnsibleAWSModule, resource_arn: str) -> Dict[str, str]:
     """
     Returns tags for provided RDS resource, formatted as an Ansible dict.
@@ -542,6 +586,44 @@ def arg_spec_to_rds_params(options_dict: Dict[str, Any]) -> Dict[str, Any]:
     return camel_options
 
 
+def format_rds_client_method_parameters(
+    client, module: AnsibleAWSModule, parameters: Dict[str, Any], method_name: str, format_tags: bool
+) -> Dict[str, Any]:
+    """
+    Returns a dict of parameters validated and formatted for the provided boto3 client method.
+
+    Performs the following parameters checks and updates:
+        - Converts parameters supplied as snake_cased module options to CamelCase
+        - Ensures that all required parameters for the provided method are present
+        - Ensures that only parameters allowed for the provided method are present, removing any that are not relevant
+        - Removes parameters with None values
+        - If format_tags is True, converts "Tags" param from an Ansible dict to boto3 list of dicts
+
+        Parameters:
+            client: boto3 rds client
+            module: AnsibleAWSModule
+            parameters (dict): Parameter options as provided to module
+            method_name (str): boto3 client method for which to validate parameters
+            format_tags (bool): Whether to convert tags from an Ansible dict to boto3 list of dicts
+
+        Returns:
+            Dict of client parameters formatted for the provided method
+
+        Raises:
+            Fails the module if any parameters required by the provided method are not provided in module options
+    """
+    required_options = get_boto3_client_method_parameters(client, method_name, required=True)
+    if any(parameters.get(k) is None for k in required_options):
+        method_description = get_rds_method_attribute(method_name, module).operation_description
+        module.fail_json(msg=f"To {method_description} requires the parameters: {required_options}")
+    options = get_boto3_client_method_parameters(client, method_name)
+    parameters = dict((k, v) for k, v in parameters.items() if k in options and v is not None)
+    if format_tags and parameters.get("Tags"):
+        parameters["Tags"] = ansible_dict_to_boto3_tag_list(parameters["Tags"])
+
+    return parameters
+
+
 def ensure_tags(
     client,
     module: AnsibleAWSModule,

plugins/modules/rds_cluster_snapshot.py

@@ -220,122 +220,102 @@
         }
 """
 
-try:
-    import botocore
-except ImportError:
-    pass  # caught by AnsibleAWSModule
+from typing import Any
+from typing import Dict
 
 from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict
 
-from ansible_collections.amazon.aws.plugins.module_utils.botocore import get_boto3_client_method_parameters
-from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code
 from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule
+from ansible_collections.amazon.aws.plugins.module_utils.rds import AnsibleRDSError
 from ansible_collections.amazon.aws.plugins.module_utils.rds import arg_spec_to_rds_params
 from ansible_collections.amazon.aws.plugins.module_utils.rds import call_method
 from ansible_collections.amazon.aws.plugins.module_utils.rds import ensure_tags
-from ansible_collections.amazon.aws.plugins.module_utils.rds import get_rds_method_attribute
-from ansible_collections.amazon.aws.plugins.module_utils.rds import get_tags
-from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry
-from ansible_collections.amazon.aws.plugins.module_utils.tagging import ansible_dict_to_boto3_tag_list
+from ansible_collections.amazon.aws.plugins.module_utils.rds import format_rds_client_method_parameters
+from ansible_collections.amazon.aws.plugins.module_utils.rds import get_snapshot
 
 
-def get_snapshot(snapshot_id):
-    try:
-        snapshot = client.describe_db_cluster_snapshots(DBClusterSnapshotIdentifier=snapshot_id, aws_retry=True)[
-            "DBClusterSnapshots"
-        ][0]
-        snapshot["Tags"] = get_tags(client, module, snapshot["DBClusterSnapshotArn"])
-    except is_boto3_error_code("DBClusterSnapshotNotFound"):
-        return {}
-    except is_boto3_error_code("DBClusterSnapshotNotFoundFault"):  # pylint: disable=duplicate-except
-        return {}
-    except (
-        botocore.exceptions.BotoCoreError,
-        botocore.exceptions.ClientError,
-    ) as e:  # pylint: disable=duplicate-except
-        module.fail_json_aws(e, msg=f"Couldn't get snapshot {snapshot_id}")
-    return snapshot
-
-
-def get_parameters(parameters, method_name):
-    if method_name == "copy_db_cluster_snapshot":
-        parameters["TargetDBClusterSnapshotIdentifier"] = module.params["db_cluster_snapshot_identifier"]
-
-    required_options = get_boto3_client_method_parameters(client, method_name, required=True)
-    if any(parameters.get(k) is None for k in required_options):
-        attribute_description = get_rds_method_attribute(method_name, module).operation_description
-        module.fail_json(msg=f"To {attribute_description} requires the parameters: {required_options}")
-    options = get_boto3_client_method_parameters(client, method_name)
-    parameters = dict((k, v) for k, v in parameters.items() if k in options and v is not None)
-
-    return parameters
-
-
-def ensure_snapshot_absent():
-    snapshot_name = module.params.get("db_cluster_snapshot_identifier")
-    params = {"DBClusterSnapshotIdentifier": snapshot_name}
+def ensure_snapshot_absent(client, module: AnsibleAWSModule) -> None:
+    snapshot_id = module.params.get("db_cluster_snapshot_identifier")
     changed = False
 
-    snapshot = get_snapshot(snapshot_name)
-    if not snapshot:
-        module.exit_json(changed=changed)
-    elif snapshot and snapshot["Status"] != "deleting":
-        snapshot, changed = call_method(client, module, "delete_db_cluster_snapshot", params)
+    try:
+        snapshot = get_snapshot(client, snapshot_id, "cluster")
+    except AnsibleRDSError as e:
+        module.fail_json_aws(e, msg=f"Failed to get snapshot: {snapshot_id}")
+    if snapshot and snapshot["Status"] != "deleting":
+        snapshot, changed = call_method(
+            client, module, "delete_db_cluster_snapshot", {"DBClusterSnapshotIdentifier": snapshot_id}
+        )
 
     module.exit_json(changed=changed)
 
 
-def copy_snapshot(params):
+def copy_snapshot(client, module: AnsibleAWSModule, params: Dict[str, Any]) -> bool:
     changed = False
     snapshot_id = module.params.get("db_cluster_snapshot_identifier")
-    snapshot = get_snapshot(snapshot_id)
 
+    try:
+        snapshot = get_snapshot(client, snapshot_id, "cluster")
+    except AnsibleRDSError as e:
+        module.fail_json_aws(e, msg=f"Failed to get snapshot: {snapshot_id}")
     if not snapshot:
-        method_params = get_parameters(params, "copy_db_cluster_snapshot")
-        if method_params.get("Tags"):
-            method_params["Tags"] = ansible_dict_to_boto3_tag_list(method_params["Tags"])
+        params["TargetDBClusterSnapshotIdentifier"] = snapshot_id
+        method_params = format_rds_client_method_parameters(
+            client, module, params, "copy_db_cluster_snapshot", format_tags=True
+        )
         _result, changed = call_method(client, module, "copy_db_cluster_snapshot", method_params)
 
     return changed
 
 
-def ensure_snapshot_present(params):
+def ensure_snapshot_present(client, module: AnsibleAWSModule, params: Dict[str, Any]) -> None:
     source_id = module.params.get("source_db_cluster_snapshot_identifier")
-    snapshot_name = module.params.get("db_cluster_snapshot_identifier")
+    snapshot_id = module.params.get("db_cluster_snapshot_identifier")
     changed = False
 
-    snapshot = get_snapshot(snapshot_name)
+    try:
+        snapshot = get_snapshot(client, snapshot_id, "cluster")
+    except AnsibleRDSError as e:
+        module.fail_json_aws(e, msg=f"Failed to get snapshot: {snapshot_id}")
 
     # Copy snapshot
     if source_id:
-        changed |= copy_snapshot(params)
+        changed |= copy_snapshot(client, module, params)
 
     # Create snapshot
     elif not snapshot:
-        changed |= create_snapshot(params)
+        changed |= create_snapshot(client, module, params)
 
     # Snapshot exists and we're not creating a copy - modify exising snapshot
     else:
-        changed |= modify_snapshot()
+        changed |= modify_snapshot(client, module)
+
+    try:
+        snapshot = get_snapshot(client, snapshot_id, "cluster")
+    except AnsibleRDSError as e:
+        module.fail_json_aws(e, msg=f"Failed to get snapshot: {snapshot_id}")
 
-    snapshot = get_snapshot(snapshot_name)
     module.exit_json(changed=changed, **camel_dict_to_snake_dict(snapshot, ignore_list=["Tags"]))
 
 
-def create_snapshot(params):
-    method_params = get_parameters(params, "create_db_cluster_snapshot")
-    if method_params.get("Tags"):
-        method_params["Tags"] = ansible_dict_to_boto3_tag_list(method_params["Tags"])
+def create_snapshot(client, module: AnsibleAWSModule, params: Dict[str, Any]) -> bool:
+    method_params = format_rds_client_method_parameters(
+        client, module, params, "create_db_cluster_snapshot", format_tags=True
+    )
     _snapshot, changed = call_method(client, module, "create_db_cluster_snapshot", method_params)
 
     return changed
 
 
-def modify_snapshot():
+def modify_snapshot(client, module: AnsibleAWSModule) -> bool:
     # TODO - add other modifications aside from purely tags
     changed = False
     snapshot_id = module.params.get("db_cluster_snapshot_identifier")
-    snapshot = get_snapshot(snapshot_id)
+
+    try:
+        snapshot = get_snapshot(client, snapshot_id, "cluster")
+    except AnsibleRDSError as e:
+        module.fail_json_aws(e, msg=f"Failed to get snapshot: {snapshot_id}")
 
     if module.params.get("tags"):
         changed |= ensure_tags(
@@ -351,9 +331,6 @@ def modify_snapshot():
 
 
 def main():
-    global client
-    global module
-
     argument_spec = dict(
         state=dict(type="str", choices=["present", "absent"], default="present"),
         db_cluster_snapshot_identifier=dict(type="str", aliases=["id", "snapshot_id", "snapshot_name"], required=True),
@@ -371,20 +348,14 @@ def main():
         argument_spec=argument_spec,
         supports_check_mode=True,
     )
-
-    retry_decorator = AWSRetry.jittered_backoff(retries=10)
-    try:
-        client = module.client("rds", retry_decorator=retry_decorator)
-    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-        module.fail_json_aws(e, msg="Failed to connect to AWS.")
-
+    client = module.client("rds")
     state = module.params.get("state")
 
     if state == "absent":
-        ensure_snapshot_absent()
+        ensure_snapshot_absent(client, module)
     elif state == "present":
         params = arg_spec_to_rds_params(dict((k, module.params[k]) for k in module.params if k in argument_spec))
-        ensure_snapshot_present(params)
+        ensure_snapshot_present(client, module, params)
 
 
 if __name__ == "__main__":