Refactor rds instance (ansible-collections#2119)

SUMMARY This is an initial refactor PR for rds modules, focusing on documentation and shared boto3 client functionality in rds module_utils, rds_instance_info, and rds_instance. First PR for ansible-collections#2003 / https://issues.redhat.com/browse/ACA-1343. COMPONENT NAME rds_instance_info rds_instance module_utils/rds.py ADDITIONAL INFORMATION Detailed summary of all the changes: module_utils/rds.py: Add RDS error class and handler Add describe_db_instances(), describe_db_snapshots(), and list_tags_for_resource() functions to handle boto3 client call Refactor get_tags() to use new list_tags_for_resource() function Add type hinting and function docstrings rds_instance_info module: Replace internal error handler and _describe_db_instances() functions with calls to new functions from module_utils/rds.py Remove extra boto3 call to retrieve tags for resource and just reformat instance TagList attribute since it is always returned by describe_db_instances Update instance_info() function return value for clarity Add type hinting and function docstrings Remove unit tests for functions no longer in module Refactor remaining unit tests to match updated instance_info() function rds_instance module: Refactor internal get_instance() function to use describe_db_instances() from module_utils/rds.py, remove extra boto3 call to get resource tags, and remove manual retry logic Refactor internal get_final_snapshot() function to use describe_db_snapshots() from module_utils/rds.py Add type hinting and function docstrings, and in some cases inline comments to explain complex logic Add unit tests for refactored functions Reviewed-by: Alina Buzachis Reviewed-by: Helen Bailey <[email protected]> Reviewed-by: Mandar Kulkarni <[email protected]> Reviewed-by: Mike Graves <[email protected]> Reviewed-by: Bikouo Aubin Reviewed-by: Mark Chappell
alinabuzachis · Jun 26, 2024 · 5874643 · 5874643
1 parent 7edc0ef
commit 5874643
Show file tree

Hide file tree

Showing 6 changed files with 677 additions and 199 deletions.
diff --git a/changelogs/fragments/refactor_rds_instance.yml b/changelogs/fragments/refactor_rds_instance.yml
@@ -0,0 +1,5 @@
+---
+minor_changes:
+  - module_utils/rds.py - Refactor shared boto3 client functionality, add type hinting and function docstrings (https://github.com/ansible-collections/amazon.aws/pull/2119).
+  - rds_instance - Refactor shared boto3 client functionality, add type hinting and function docstrings (https://github.com/ansible-collections/amazon.aws/pull/2119).
+  - rds_instance_info - Refactor shared boto3 client functionality, add type hinting and function docstrings (https://github.com/ansible-collections/amazon.aws/pull/2119).
diff --git a/plugins/module_utils/rds.py b/plugins/module_utils/rds.py
@@ -8,6 +8,8 @@
 from typing import Any
 from typing import Dict
 from typing import List
+from typing import Optional
+from typing import Tuple
 
 try:
     from botocore.exceptions import BotoCoreError
@@ -21,6 +23,8 @@
 
 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
@@ -83,7 +87,39 @@
 ]
 
 
-def get_rds_method_attribute(method_name, module):
+class AnsibleRDSError(AnsibleAWSError):
+    pass
+
+
+class RDSErrorHandler(AWSErrorHandler):
+    _CUSTOM_EXCEPTION = AnsibleRDSError
+
+    @classmethod
+    def _is_missing(cls):
+        return is_boto3_error_code(["DBInstanceNotFound", "DBSnapshotNotFound", "DBClusterNotFound"])
+
+
+@RDSErrorHandler.list_error_handler("describe db instances", [])
+@AWSRetry.jittered_backoff()
+def describe_db_instances(client, **params: Dict) -> List[Dict[str, Any]]:
+    paginator = client.get_paginator("describe_db_instances")
+    return paginator.paginate(**params).build_full_result()["DBInstances"]
+
+
+@RDSErrorHandler.list_error_handler("describe db snapshots", [])
+@AWSRetry.jittered_backoff()
+def describe_db_snapshots(client, **params: Dict) -> List[Dict]:
+    paginator = client.get_paginator("describe_db_snapshots")
+    return paginator.paginate(**params).build_full_result()["DBSnapshots"]
+
+
+@RDSErrorHandler.list_error_handler("list tags for resource", [])
+@AWSRetry.jittered_backoff()
+def list_tags_for_resource(client, resource_arn: str) -> List[Dict[str, str]]:
+    return client.list_tags_for_resource(ResourceName=resource_arn)["TagList"]
+
+
+def get_rds_method_attribute(method_name: str, module: AnsibleAWSModule) -> Boto3ClientMethod:
     """
     Returns rds attributes of the specified method.
 
@@ -174,7 +210,21 @@ def get_rds_method_attribute(method_name, module):
     )
 
 
-def get_final_identifier(method_name, module):
+def get_final_identifier(method_name: str, module: AnsibleAWSModule) -> str:
+    """
+    Returns the final identifier for the resource to which the specified method applies.
+
+        Parameters:
+            method_name (str): RDS method whose target resource final identifier is returned
+            module: AnsibleAWSModule
+
+        Returns:
+            updated_identifier (str): The new resource identifier from module params if not in check mode, there is a new identifier in module params, and
+                apply_immediately is True; otherwise returns the original resource identifier from module params
+
+        Raises:
+            NotImplementedError if the provided method is not supported
+    """
     updated_identifier = None
     apply_immediately = module.params.get("apply_immediately")
     resource = get_rds_method_attribute(method_name, module).resource
@@ -197,7 +247,20 @@ def get_final_identifier(method_name, module):
     return identifier
 
 
-def handle_errors(module, exception, method_name, parameters):
+def handle_errors(module: AnsibleAWSModule, exception: Any, method_name: str, parameters: Dict[str, Any]) -> bool:
+    """
+    Fails the module with an appropriate error message given the provided exception.
+
+        Parameters:
+            module: AnsibleAWSModule
+            exception: Botocore exception to be handled
+            method_name (str): Name of boto3 rds client method
+            parameters (dict): Parameters provided to boto3 client method
+
+        Returns:
+            changed (bool): False if provided exception indicates that no modifications were requested or a read replica promotion was attempted on an
+                instance/cluseter that is not a read replica; should never return True (the module should always fail instead)
+    """
     if not isinstance(exception, ClientError):
         module.fail_json_aws(exception, msg=f"Unexpected failure for method {method_name} with parameters {parameters}")
 
@@ -252,7 +315,23 @@ def handle_errors(module, exception, method_name, parameters):
     return changed
 
 
-def call_method(client, module, method_name, parameters):
+def call_method(client, module: AnsibleAWSModule, method_name: str, parameters: Dict[str, Any]) -> Tuple[Any, bool]:
+    """Calls the provided boto3 rds client method with the provided parameters.
+
+    Handles check mode determination, whether or not to wait for resource status, and method-specific retry codes.
+
+        Parameters:
+            client: boto3 rds client
+            module: Ansible AWS module
+            method_name (str): Name of the boto3 rds client method to call
+            parameters (dict): Parameters to pass to the boto3 client method; these must already match expected parameters for the method and
+                be formatted correctly (CamelCase, Tags and other attributes converted to lists of dicts as needed)
+
+        Returns:
+            tuple (any, bool):
+                result (any): Result value from method call
+                changed (bool): True if changes were made to the resource, False otherwise
+    """
     result = {}
     changed = True
     if not module.check_mode:
@@ -270,7 +349,19 @@ def call_method(client, module, method_name, parameters):
     return result, changed
 
 
-def wait_for_instance_status(client, module, db_instance_id, waiter_name):
+def wait_for_instance_status(client, module: AnsibleAWSModule, db_instance_id: str, waiter_name: str) -> None:
+    """
+    Waits until provided instance has reached the expected status for provided waiter.
+
+    Fails the module if an exception is raised while waiting.
+
+        Parameters:
+            client: boto3 rds client
+            module: AnsibleAWSModule
+            db_instance_id (str): DB instance identifier
+            waiter_name (str): Name of either a boto3 rds client waiter or an RDS waiter defined in module_utils/waiters.py
+    """
+
     def wait(client, db_instance_id, waiter_name):
         try:
             waiter = client.get_waiter(waiter_name)
@@ -300,7 +391,18 @@ def wait(client, db_instance_id, waiter_name):
             )
 
 
-def wait_for_cluster_status(client, module, db_cluster_id, waiter_name):
+def wait_for_cluster_status(client, module: AnsibleAWSModule, db_cluster_id: str, waiter_name: str) -> None:
+    """
+    Waits until provided cluster has reached the expected status for provided waiter.
+
+    Fails the module if an exception is raised while waiting.
+
+        Parameters:
+            client: boto3 rds client
+            module: AnsibleAWSModule
+            db_cluster_id (str): DB cluster identifier
+            waiter_name (str): Name of either a boto3 rds client waiter or an RDS waiter defined in module_utils/waiters.py
+    """
     try:
         get_waiter(client, waiter_name).wait(DBClusterIdentifier=db_cluster_id)
     except WaiterError as e:
@@ -313,7 +415,18 @@ def wait_for_cluster_status(client, module, db_cluster_id, waiter_name):
         module.fail_json_aws(e, msg=f"Failed with an unexpected error while waiting for the DB cluster {db_cluster_id}")
 
 
-def wait_for_instance_snapshot_status(client, module, db_snapshot_id, waiter_name):
+def wait_for_instance_snapshot_status(client, module: AnsibleAWSModule, db_snapshot_id: str, waiter_name: str) -> None:
+    """
+    Waits until provided instance snapshot has reached the expected status for provided waiter.
+
+    Fails the module if an exception is raised while waiting.
+
+        Parameters:
+            client: boto3 rds client
+            module: AnsibleAWSModule
+            db_snapshot_id (str): DB instance snapshot identifier
+            waiter_name (str): Name of a boto3 rds client waiter
+    """
     try:
         client.get_waiter(waiter_name).wait(DBSnapshotIdentifier=db_snapshot_id)
     except WaiterError as e:
@@ -328,7 +441,18 @@ def wait_for_instance_snapshot_status(client, module, db_snapshot_id, waiter_nam
         )
 
 
-def wait_for_cluster_snapshot_status(client, module, db_snapshot_id, waiter_name):
+def wait_for_cluster_snapshot_status(client, module: AnsibleAWSModule, db_snapshot_id: str, waiter_name: str) -> None:
+    """
+    Waits until provided cluster snapshot has reached the expected status for provided waiter.
+
+    Fails the module if an exception is raised while waiting.
+
+        Parameters:
+            client: boto3 rds client
+            module: AnsibleAWSModule
+            db_snapshot_id (str): DB cluster snapshot identifier
+            waiter_name (str): Name of a boto3 rds client waiter
+    """
     try:
         client.get_waiter(waiter_name).wait(DBClusterSnapshotIdentifier=db_snapshot_id)
     except WaiterError as e:
@@ -344,7 +468,16 @@ def wait_for_cluster_snapshot_status(client, module, db_snapshot_id, waiter_name
         )
 
 
-def wait_for_status(client, module, identifier, method_name):
+def wait_for_status(client, module: AnsibleAWSModule, identifier: str, method_name: str) -> None:
+    """
+    Waits until provided resource has reached the expected final status for provided method.
+
+        Parameters:
+            client: boto3 rds client
+            module: AnsibleAWSModule
+            identifier (str): resource identifier
+            method_name (str): Name of boto3 rds client method on whose final status to wait
+    """
     rds_method_attributes = get_rds_method_attribute(method_name, module)
     waiter_name = rds_method_attributes.waiter
     resource = rds_method_attributes.resource
@@ -359,14 +492,40 @@ def wait_for_status(client, module, identifier, method_name):
         wait_for_cluster_snapshot_status(client, module, identifier, waiter_name)
 
 
-def get_tags(client, module, resource_arn):
+def get_tags(client, module: AnsibleAWSModule, resource_arn: str) -> Dict[str, str]:
+    """
+    Returns tags for provided RDS resource, formatted as an Ansible dict.
+
+    Fails the module if an error is raised while retrieving resource tags.
+
+        Parameters:
+            client: boto3 rds client
+            module: AnsibleAWSModule
+            resource_arn (str): AWS resource ARN
+
+        Returns:
+            tags (dict): Tags for resource, formatted as an Ansible dict. An empty list is returned if the resource has no tags.
+    """
     try:
-        return boto3_tag_list_to_ansible_dict(client.list_tags_for_resource(ResourceName=resource_arn)["TagList"])
-    except (BotoCoreError, ClientError) as e:
-        module.fail_json_aws(e, msg="Unable to describe tags")
+        tags = list_tags_for_resource(client, resource_arn)
+    except AnsibleRDSError as e:
+        module.fail_json_aws(e, msg=f"Unable to list tags for resource {resource_arn}")
+    return boto3_tag_list_to_ansible_dict(tags)
 
 
-def arg_spec_to_rds_params(options_dict):
+def arg_spec_to_rds_params(options_dict: Dict[str, Any]) -> Dict[str, Any]:
+    """
+    Converts snake_cased rds module options to CamelCased parameter formats expected by boto3 rds client.
+
+    Does not alter case for keys or values in the following attributes: tags, processor_features.
+    Includes special handling of certain boto3 params that do not follow standard CamelCase.
+
+        Parameters:
+            options_dict (dict): Snake-cased options for a boto3 rds client method
+
+        Returns:
+            camel_options (dct): Options formatted for boto3 rds client
+    """
     tags = options_dict.pop("tags")
     has_processor_features = False
     if "processor_features" in options_dict:
@@ -383,7 +542,30 @@ def arg_spec_to_rds_params(options_dict):
     return camel_options
 
 
-def ensure_tags(client, module, resource_arn, existing_tags, tags, purge_tags):
+def ensure_tags(
+    client,
+    module: AnsibleAWSModule,
+    resource_arn: str,
+    existing_tags: Dict[str, str],
+    tags: Optional[Dict[str, str]],
+    purge_tags: bool,
+) -> bool:
+    """
+    Compares current resource tages to desired tags and adds/removes tags to ensure desired tags are present.
+
+    A value of None for desired tags results in resource tags being left as is.
+
+        Parameters:
+            client: boto3 rds client
+            module: AnsibleAWSModule
+            resource_arn (str): AWS resource ARN
+            existing_tags (dict): Current resource tags formatted as an Ansible dict
+            tags (dict): Desired resource tags formatted as an Ansible dict
+            purge_tags (bool): Whether to remove any existing resource tags not present in desired tags
+
+        Returns:
+            True if resource tags are updated, False if not.
+    """
     if tags is None:
         return False
     tags_to_add, tags_to_remove = compare_aws_tags(existing_tags, tags, purge_tags)
@@ -405,13 +587,15 @@ def ensure_tags(client, module, resource_arn, existing_tags, tags, purge_tags):
     return changed
 
 
-def compare_iam_roles(existing_roles, target_roles, purge_roles):
+def compare_iam_roles(
+    existing_roles: List[Dict[str, str]], target_roles: List[Dict[str, str]], purge_roles: bool
+) -> Tuple[List[Dict[str, str]], List[Dict[str, str]]]:
     """
-    Returns differences between target and existing IAM roles
+    Returns differences between target and existing IAM roles.
 
         Parameters:
-            existing_roles (list): Existing IAM roles
-            target_roles (list): Target IAM roles
+            existing_roles (list): Existing IAM roles as a list of snake-cased dicts
+            target_roles (list): Target IAM roles as a list of snake-cased dicts
             purge_roles (bool): Remove roles not in target_roles if True
 
         Returns:
@@ -424,16 +608,22 @@ def compare_iam_roles(existing_roles, target_roles, purge_roles):
     return roles_to_add, roles_to_remove
 
 
-def update_iam_roles(client, module, instance_id, roles_to_add, roles_to_remove):
+def update_iam_roles(
+    client,
+    module: AnsibleAWSModule,
+    instance_id: str,
+    roles_to_add: List[Dict[str, str]],
+    roles_to_remove: List[Dict[str, str]],
+) -> bool:
     """
     Update a DB instance's associated IAM roles
 
         Parameters:
             client: RDS client
             module: AnsibleAWSModule
             instance_id (str): DB's instance ID
-            roles_to_add (list): List of IAM roles to add
-            roles_to_delete (list): List of IAM roles to delete
+            roles_to_add (list): List of IAM roles to add in snake-cased dict format
+            roles_to_delete (list): List of IAM roles to delete in snake-cased dict format
 
         Returns:
             changed (bool): True if changes were successfully made to DB instance's IAM roles; False if not
@@ -449,7 +639,7 @@ def update_iam_roles(client, module, instance_id, roles_to_add, roles_to_remove)
 
 @AWSRetry.jittered_backoff()
 def describe_db_cluster_parameter_groups(
-    module: AnsibleAWSModule, connection: Any, group_name: str
+    module: AnsibleAWSModule, connection: Any, group_name: Optional[str]
 ) -> List[Dict[str, Any]]:
     result = []
     try: