rds module_util - fix check_mode and idempotence bugs and support add…

…ing/removing IAM roles (ansible-collections#714) rds module_util - fix check_mode and idempotence bugs and support adding/removing IAM roles SUMMARY Add waiter for promoting read replica to fix idempotence bug in community.aws integration testing Support modifying IAM roles to a db instance for ansible-collections/community.aws#1002 Add necessary waiters for both adding and removing an IAM role compare and ensure iam_roles methods for idempotency unit & integration tests for coverage (integration tests in community.aws PR) ISSUE TYPE Feature Pull Request COMPONENT NAME module_util/rds community.aws.rds_instance ADDITIONAL INFORMATION Refs: (adding role) https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/add-role-to-db-instance.html (removing role) https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/remove-role-from-db-instance.html (waiters) https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/describe-db-instances.html Reviewed-by: Alina Buzachis <None> Reviewed-by: Joseph Torcasso <None> Reviewed-by: Mark Woolley <[email protected]> Reviewed-by: Mandar Kulkarni <[email protected]> Reviewed-by: Jill R <None> Reviewed-by: Mike Graves <[email protected]> (cherry picked from commit 00c752e)
jatorcasso · Apr 12, 2022 · ff04d42 · ff04d42
1 parent a27a3fa
commit ff04d42
Show file tree

Hide file tree

Showing 4 changed files with 221 additions and 22 deletions.
diff --git a/changelogs/fragments/714-module_util_rds-support-iam-roles-add-waiters.yml b/changelogs/fragments/714-module_util_rds-support-iam-roles-add-waiters.yml
@@ -0,0 +1,4 @@
+minor_changes:
+  - module_utils.rds - Add support and unit tests for addition/removal of IAM roles to/from a db instance in module_utils.rds with waiters (https://github.com/ansible-collections/amazon.aws/pull/714).
+bugfixes:
+  - module.utils.rds - Add waiter for promoting read replica to fix idempotency issue (https://github.com/ansible-collections/amazon.aws/pull/714).
diff --git a/plugins/module_utils/rds.py b/plugins/module_utils/rds.py
@@ -32,7 +32,8 @@
     'create_db_instance', 'restore_db_instance_to_point_in_time', 'restore_db_instance_from_s3',
     'restore_db_instance_from_db_snapshot', 'create_db_instance_read_replica', 'modify_db_instance',
     'delete_db_instance', 'add_tags_to_resource', 'remove_tags_from_resource', 'list_tags_for_resource',
-    'promote_read_replica', 'stop_db_instance', 'start_db_instance', 'reboot_db_instance'
+    'promote_read_replica', 'stop_db_instance', 'start_db_instance', 'reboot_db_instance', 'add_role_to_db_instance',
+    'remove_role_from_db_instance'
 ]
 
 snapshot_cluster_method_names = [
@@ -65,6 +66,12 @@ def get_rds_method_attribute(method_name, module):
             waiter = 'db_instance_deleted'
         elif method_name == 'stop_db_instance':
             waiter = 'db_instance_stopped'
+        elif method_name == 'add_role_to_db_instance':
+            waiter = 'role_associated'
+        elif method_name == 'remove_role_from_db_instance':
+            waiter = 'role_disassociated'
+        elif method_name == 'promote_read_replica':
+            waiter = 'read_replica_promoted'
         else:
             waiter = 'db_instance_available'
     elif method_name in snapshot_cluster_method_names or method_name in snapshot_instance_method_names:
@@ -130,15 +137,6 @@ def handle_errors(module, exception, method_name, parameters):
             changed = False
         else:
             module.fail_json_aws(exception, msg='Unable to {0}'.format(get_rds_method_attribute(method_name, module).operation_description))
-    elif method_name == 'create_db_instance' and error_code == 'InvalidParameterValue':
-        accepted_engines = [
-            'aurora', 'aurora-mysql', 'aurora-postgresql', 'mariadb', 'mysql', 'oracle-ee', 'oracle-se',
-            'oracle-se1', 'oracle-se2', 'postgres', 'sqlserver-ee', 'sqlserver-ex', 'sqlserver-se', 'sqlserver-web'
-        ]
-        if parameters.get('Engine') not in accepted_engines:
-            module.fail_json_aws(exception, msg='DB engine {0} should be one of {1}'.format(parameters.get('Engine'), accepted_engines))
-        else:
-            module.fail_json_aws(exception, msg='Unable to {0}'.format(get_rds_method_attribute(method_name, module).operation_description))
     elif method_name == 'create_db_cluster' and error_code == 'InvalidParameterValue':
         accepted_engines = [
             'aurora', 'aurora-mysql', 'aurora-postgresql'
@@ -161,10 +159,10 @@ def call_method(client, module, method_name, parameters):
         # TODO: stabilize by adding get_rds_method_attribute(method_name).extra_retry_codes
         method = getattr(client, method_name)
         try:
-            if method_name == 'modify_db_instance':
+            if method_name in ['modify_db_instance', 'promote_read_replica']:
                 # check if instance is in an available state first, if possible
                 if wait:
-                    wait_for_status(client, module, module.params['db_instance_identifier'], method_name)
+                    wait_for_status(client, module, module.params['db_instance_identifier'], 'modify_db_instance')
                 result = AWSRetry.jittered_backoff(catch_extra_error_codes=['InvalidDBInstanceState'])(method)(**parameters)
             elif method_name == 'modify_db_cluster':
                 # check if cluster is in an available state first, if possible
@@ -305,3 +303,49 @@ def ensure_tags(client, module, resource_arn, existing_tags, tags, purge_tags):
             parameters={'ResourceName': resource_arn, 'TagKeys': tags_to_remove}
         )
     return changed
+
+
+def compare_iam_roles(existing_roles, target_roles, purge_roles):
+    '''
+    Returns differences between target and existing IAM roles
+
+        Parameters:
+            existing_roles (list): Existing IAM roles
+            target_roles (list): Target IAM roles
+            purge_roles (bool): Remove roles not in target_roles if True
+
+        Returns:
+            roles_to_add (list): List of IAM roles to add
+            roles_to_delete (list): List of IAM roles to delete
+    '''
+    existing_roles = [dict((k, v) for k, v in role.items() if k != 'status') for role in existing_roles]
+    roles_to_add = [role for role in target_roles if role not in existing_roles]
+    roles_to_remove = [role for role in existing_roles if role not in target_roles] if purge_roles else []
+    return roles_to_add, roles_to_remove
+
+
+def update_iam_roles(client, module, instance_id, roles_to_add, roles_to_remove):
+    '''
+    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
+
+        Returns:
+            changed (bool): True if changes were successfully made to DB instance's IAM roles; False if not
+    '''
+    for role in roles_to_remove:
+        params = {'DBInstanceIdentifier': instance_id,
+                  'RoleArn': role['role_arn'],
+                  'FeatureName': role['feature_name']}
+        result, changed = call_method(client, module, method_name='remove_role_from_db_instance', parameters=params)
+    for role in roles_to_add:
+        params = {'DBInstanceIdentifier': instance_id,
+                  'RoleArn': role['role_arn'],
+                  'FeatureName': role['feature_name']}
+        result, changed = call_method(client, module, method_name='add_role_to_db_instance', parameters=params)
+    return changed
diff --git a/plugins/module_utils/waiters.py b/plugins/module_utils/waiters.py
@@ -648,6 +648,69 @@
                 }
             ]
         },
+        "ReadReplicaPromoted": {
+            "delay": 5,
+            "maxAttempts": 40,
+            "operation": "DescribeDBInstances",
+            "acceptors": [
+                {
+                    "state": "success",
+                    "matcher": "path",
+                    "argument": "length(DBInstances[].StatusInfos) == `0`",
+                    "expected": True
+                },
+                {
+                    "state": "retry",
+                    "matcher": "pathAny",
+                    "argument": "DBInstances[].StatusInfos[].Status",
+                    "expected": "replicating"
+                }
+            ]
+        },
+        "RoleAssociated": {
+            "delay": 5,
+            "maxAttempts": 40,
+            "operation": "DescribeDBInstances",
+            "acceptors": [
+                {
+                    "state": "success",
+                    "matcher": "pathAll",
+                    "argument": "DBInstances[].AssociatedRoles[].Status",
+                    "expected": "ACTIVE"
+                },
+                {
+                    "state": "retry",
+                    "matcher": "pathAny",
+                    "argument": "DBInstances[].AssociatedRoles[].Status",
+                    "expected": "PENDING"
+                }
+            ]
+        },
+        "RoleDisassociated": {
+            "delay": 5,
+            "maxAttempts": 40,
+            "operation": "DescribeDBInstances",
+            "acceptors": [
+                {
+                    "state": "success",
+                    "matcher": "pathAll",
+                    "argument": "DBInstances[].AssociatedRoles[].Status",
+                    "expected": "ACTIVE"
+                },
+                {
+                    "state": "retry",
+                    "matcher": "pathAny",
+                    "argument": "DBInstances[].AssociatedRoles[].Status",
+                    "expected": "PENDING"
+                },
+                {
+                    "state": "success",
+                    "matcher": "path",
+                    "argument": "length(DBInstances[].AssociatedRoles[]) == `0`",
+                    "expected": True
+                },
+            ]
+        }
     }
 }
 
@@ -944,6 +1007,24 @@ def route53_model(name):
         core_waiter.NormalizedOperationMethod(
             rds.describe_db_clusters
         )),
+    ('RDS', 'read_replica_promoted'): lambda rds: core_waiter.Waiter(
+        'read_replica_promoted',
+        rds_model('ReadReplicaPromoted'),
+        core_waiter.NormalizedOperationMethod(
+            rds.describe_db_instances
+        )),
+    ('RDS', 'role_associated'): lambda rds: core_waiter.Waiter(
+        'role_associated',
+        rds_model('RoleAssociated'),
+        core_waiter.NormalizedOperationMethod(
+            rds.describe_db_instances
+        )),
+    ('RDS', 'role_disassociated'): lambda rds: core_waiter.Waiter(
+        'role_disassociated',
+        rds_model('RoleDisassociated'),
+        core_waiter.NormalizedOperationMethod(
+            rds.describe_db_instances
+        )),
     ('Route53', 'resource_record_sets_changed'): lambda route53: core_waiter.Waiter(
         'resource_record_sets_changed',
         route53_model('ResourceRecordSetsChanged'),

diff --git a/tests/unit/module_utils/test_rds.py b/tests/unit/module_utils/test_rds.py
@@ -7,9 +7,9 @@
 
 __metaclass__ = type
 
-from ansible_collections.amazon.aws.tests.unit.compat.mock import MagicMock
 from ansible_collections.amazon.aws.plugins.module_utils import rds
-
+from ansible_collections.amazon.aws.tests.unit.compat import unittest
+from ansible_collections.amazon.aws.tests.unit.compat.mock import MagicMock
 
 from contextlib import nullcontext
 import pytest
@@ -284,14 +284,6 @@ def test__handle_errors(method_name, exception, expected):
                 match="method promote_read_replica_db_cluster hasn't been added to the list of accepted methods to use a waiter in module_utils/rds.py",
             ),
         ),
-        (
-            "create_db_instance",
-            build_exception("create_db_instance", code="InvalidParameterValue"),
-            *expected(
-                "DB engine fake_engine should be one of aurora, aurora-mysql, aurora-postgresql, mariadb, mysql, oracle-ee, oracle-se, oracle-se1, "
-                + "oracle-se2, postgres, sqlserver-ee, sqlserver-ex, sqlserver-se, sqlserver-web"
-            ),
-        ),
         (
             "create_db_cluster",
             build_exception("create_db_cluster", code="InvalidParameterValue"),
@@ -308,3 +300,81 @@ def test__handle_errors_failed(method_name, exception, expected, error):
         rds.handle_errors(module, exception, method_name, {"Engine": "fake_engine"})
         module.fail_json_aws.assert_called_once
         module.fail_json_aws.call_args[1]["msg"] == expected
+
+
+class RdsUtils(unittest.TestCase):
+
+    # ========================================================
+    # Setup some initial data that we can use within our tests
+    # ========================================================
+    def setUp(self):
+        self.target_role_list = [
+            {
+                'role_arn': 'role_won',
+                'feature_name': 's3Export'
+            },
+            {
+                'role_arn': 'role_too',
+                'feature_name': 'Lambda'
+            },
+            {
+                'role_arn': 'role_thrie',
+                'feature_name': 's3Import'
+            }
+        ]
+
+    # ========================================================
+    #   rds.compare_iam_roles
+    # ========================================================
+
+    def test_compare_iam_roles_equal(self):
+        existing_list = self.target_role_list
+        roles_to_add, roles_to_delete = rds.compare_iam_roles(existing_list, self.target_role_list, purge_roles=False)
+        self.assertEqual([], roles_to_add)
+        self.assertEqual([], roles_to_delete)
+        roles_to_add, roles_to_delete = rds.compare_iam_roles(existing_list, self.target_role_list, purge_roles=True)
+        self.assertEqual([], roles_to_add)
+        self.assertEqual([], roles_to_delete)
+
+    def test_compare_iam_roles_empty_arr_existing(self):
+        roles_to_add, roles_to_delete = rds.compare_iam_roles([], self.target_role_list, purge_roles=False)
+        self.assertEqual(self.target_role_list, roles_to_add)
+        self.assertEqual([], roles_to_delete)
+        roles_to_add, roles_to_delete = rds.compare_iam_roles([], self.target_role_list, purge_roles=True)
+        self.assertEqual(self.target_role_list, roles_to_add)
+        self.assertEqual([], roles_to_delete)
+
+    def test_compare_iam_roles_empty_arr_target(self):
+        existing_list = self.target_role_list
+        roles_to_add, roles_to_delete = rds.compare_iam_roles(existing_list, [], purge_roles=False)
+        self.assertEqual([], roles_to_add)
+        self.assertEqual([], roles_to_delete)
+        roles_to_add, roles_to_delete = rds.compare_iam_roles(existing_list, [], purge_roles=True)
+        self.assertEqual([], roles_to_add)
+        self.assertEqual(self.target_role_list, roles_to_delete)
+
+    def test_compare_iam_roles_different(self):
+        existing_list = [
+            {
+                'role_arn': 'role_wonn',
+                'feature_name': 's3Export'
+            }]
+        roles_to_add, roles_to_delete = rds.compare_iam_roles(existing_list, self.target_role_list, purge_roles=False)
+        self.assertEqual(self.target_role_list, roles_to_add)
+        self.assertEqual([], roles_to_delete)
+        roles_to_add, roles_to_delete = rds.compare_iam_roles(existing_list, self.target_role_list, purge_roles=True)
+        self.assertEqual(self.target_role_list, roles_to_add)
+        self.assertEqual(existing_list, roles_to_delete)
+
+        existing_list = self.target_role_list.copy()
+        self.target_role_list = [
+            {
+                'role_arn': 'role_wonn',
+                'feature_name': 's3Export'
+            }]
+        roles_to_add, roles_to_delete = rds.compare_iam_roles(existing_list, self.target_role_list, purge_roles=False)
+        self.assertEqual(self.target_role_list, roles_to_add)
+        self.assertEqual([], roles_to_delete)
+        roles_to_add, roles_to_delete = rds.compare_iam_roles(existing_list, self.target_role_list, purge_roles=True)
+        self.assertEqual(self.target_role_list, roles_to_add)
+        self.assertEqual(existing_list, roles_to_delete)