From 0eea687a30ed0c6292a3c83d3159403a49e812af Mon Sep 17 00:00:00 2001 From: Joseph Torcasso Date: Wed, 16 Mar 2022 23:01:12 -0400 Subject: [PATCH 01/11] add util methods/waiters for ensuring iam roles to db instance --- plugins/module_utils/rds.py | 50 ++++++++++++++++++++++++++++- plugins/module_utils/waiters.py | 56 +++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/plugins/module_utils/rds.py b/plugins/module_utils/rds.py index e13ea778a47..573d449c559 100644 --- 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,10 @@ 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' else: waiter = 'db_instance_available' elif method_name in snapshot_cluster_method_names or method_name in snapshot_instance_method_names: @@ -305,3 +310,46 @@ 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): + roles_to_add = [] + roles_to_remove = [] + for target_role in target_roles: + found = False + for existing_role in existing_roles: + if target_role['role_arn'] == existing_role['RoleArn'] and target_role['feature_name'] == existing_role['FeatureName']: + found = True + break + if not found: + roles_to_add.append(target_role) + + if purge_roles: + for existing_role in existing_roles: + found = False + for target_role in target_roles: + if target_role['role_arn'] == existing_role['RoleArn'] and target_role['feature_name'] == existing_role['FeatureName']: + found = True + break + if not found: + roles_to_remove.append(existing_role) + + return roles_to_add, roles_to_remove + + +def ensure_iam_roles(client, module, instance, instance_id, iam_roles, purge_iam_roles): + if iam_roles is None: + iam_roles = [] + roles_to_add, roles_to_remove = compare_iam_roles(instance['AssociatedRoles'], iam_roles, purge_iam_roles) + changed = bool(roles_to_add or roles_to_remove) + for role in roles_to_remove: + params = {'DBInstanceIdentifier': instance_id, + 'RoleArn': role['RoleArn'], + 'FeatureName': role['FeatureName']} + 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 index 881598c2601..a86ba319f53 100644 --- a/plugins/module_utils/waiters.py +++ b/plugins/module_utils/waiters.py @@ -685,6 +685,50 @@ } ] }, + "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 + }, + ] + } } } @@ -993,6 +1037,18 @@ def route53_model(name): core_waiter.NormalizedOperationMethod( rds.describe_db_clusters )), + ('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'), From 75c016d973e6185de4cf49a54c20e910e47f7a38 Mon Sep 17 00:00:00 2001 From: Joseph Torcasso Date: Wed, 16 Mar 2022 23:08:21 -0400 Subject: [PATCH 02/11] changelog --- changelogs/fragments/714-module_util_rds-support-iam-roles.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/714-module_util_rds-support-iam-roles.yml diff --git a/changelogs/fragments/714-module_util_rds-support-iam-roles.yml b/changelogs/fragments/714-module_util_rds-support-iam-roles.yml new file mode 100644 index 00000000000..f994f8cb4a1 --- /dev/null +++ b/changelogs/fragments/714-module_util_rds-support-iam-roles.yml @@ -0,0 +1,2 @@ +minor_changes: + - module_utils.rds - Add support for addition/removal of IAM roles to/from a db instance in module_utils.elbv2 with waiters (https://github.com/ansible-collections/amazon.aws/pull/714). From 0e34d2536fbd83323bb21213b28e816e00bb4cc8 Mon Sep 17 00:00:00 2001 From: Joseph Torcasso Date: Mon, 21 Mar 2022 11:01:14 -0400 Subject: [PATCH 03/11] add unit testing --- plugins/module_utils/rds.py | 17 +++--- tests/unit/module_utils/test_rds.py | 82 ++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/plugins/module_utils/rds.py b/plugins/module_utils/rds.py index 573d449c559..1e7ecfce133 100644 --- a/plugins/module_utils/rds.py +++ b/plugins/module_utils/rds.py @@ -13,6 +13,7 @@ pass from ansible.module_utils._text import to_text +from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict from ansible.module_utils.common.dict_transformations import snake_dict_to_camel_dict from .ec2 import AWSRetry @@ -318,7 +319,7 @@ def compare_iam_roles(existing_roles, target_roles, purge_roles): for target_role in target_roles: found = False for existing_role in existing_roles: - if target_role['role_arn'] == existing_role['RoleArn'] and target_role['feature_name'] == existing_role['FeatureName']: + if target_role['role_arn'] == existing_role['role_arn'] and target_role['feature_name'] == existing_role['feature_name']: found = True break if not found: @@ -328,7 +329,7 @@ def compare_iam_roles(existing_roles, target_roles, purge_roles): for existing_role in existing_roles: found = False for target_role in target_roles: - if target_role['role_arn'] == existing_role['RoleArn'] and target_role['feature_name'] == existing_role['FeatureName']: + if target_role['role_arn'] == existing_role['role_arn'] and target_role['feature_name'] == existing_role['feature_name']: found = True break if not found: @@ -337,15 +338,15 @@ def compare_iam_roles(existing_roles, target_roles, purge_roles): return roles_to_add, roles_to_remove -def ensure_iam_roles(client, module, instance, instance_id, iam_roles, purge_iam_roles): - if iam_roles is None: - iam_roles = [] - roles_to_add, roles_to_remove = compare_iam_roles(instance['AssociatedRoles'], iam_roles, purge_iam_roles) +def ensure_iam_roles(client, module, instance_id, existing_roles, target_roles, purge_iam_roles): + if target_roles is None: + target_roles = [] + roles_to_add, roles_to_remove = compare_iam_roles(existing_roles, target_roles, purge_iam_roles) changed = bool(roles_to_add or roles_to_remove) for role in roles_to_remove: params = {'DBInstanceIdentifier': instance_id, - 'RoleArn': role['RoleArn'], - 'FeatureName': role['FeatureName']} + '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, diff --git a/tests/unit/module_utils/test_rds.py b/tests/unit/module_utils/test_rds.py index 4ff02454f0a..b485763ee50 100644 --- 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 @@ -308,3 +308,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) From 2e96a740b291464853ade4460e8f8eb383f6154a Mon Sep 17 00:00:00 2001 From: Joseph Torcasso Date: Mon, 21 Mar 2022 11:03:30 -0400 Subject: [PATCH 04/11] update changelog --- changelogs/fragments/714-module_util_rds-support-iam-roles.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/fragments/714-module_util_rds-support-iam-roles.yml b/changelogs/fragments/714-module_util_rds-support-iam-roles.yml index f994f8cb4a1..f077b699589 100644 --- a/changelogs/fragments/714-module_util_rds-support-iam-roles.yml +++ b/changelogs/fragments/714-module_util_rds-support-iam-roles.yml @@ -1,2 +1,2 @@ minor_changes: - - module_utils.rds - Add support for addition/removal of IAM roles to/from a db instance in module_utils.elbv2 with waiters (https://github.com/ansible-collections/amazon.aws/pull/714). + - module_utils.rds - Add support and unit tests for addition/removal of IAM roles to/from a db instance in module_utils.elbv2 with waiters (https://github.com/ansible-collections/amazon.aws/pull/714). From ee09a4af182e1d1f13ec9e8301031b750c5224d2 Mon Sep 17 00:00:00 2001 From: Joseph Torcasso Date: Thu, 24 Mar 2022 16:34:22 -0400 Subject: [PATCH 05/11] change ensure_roles to compare_roles for check_mode/idempotence --- plugins/module_utils/rds.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/plugins/module_utils/rds.py b/plugins/module_utils/rds.py index 1e7ecfce133..fe7fcd4ef02 100644 --- a/plugins/module_utils/rds.py +++ b/plugins/module_utils/rds.py @@ -13,7 +13,6 @@ pass from ansible.module_utils._text import to_text -from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict from ansible.module_utils.common.dict_transformations import snake_dict_to_camel_dict from .ec2 import AWSRetry @@ -314,6 +313,8 @@ def ensure_tags(client, module, resource_arn, existing_tags, tags, purge_tags): def compare_iam_roles(existing_roles, target_roles, purge_roles): + if target_roles is None: + target_roles = [] roles_to_add = [] roles_to_remove = [] for target_role in target_roles: @@ -338,11 +339,7 @@ def compare_iam_roles(existing_roles, target_roles, purge_roles): return roles_to_add, roles_to_remove -def ensure_iam_roles(client, module, instance_id, existing_roles, target_roles, purge_iam_roles): - if target_roles is None: - target_roles = [] - roles_to_add, roles_to_remove = compare_iam_roles(existing_roles, target_roles, purge_iam_roles) - changed = bool(roles_to_add or roles_to_remove) +def update_iam_roles(client, module, instance_id, roles_to_add, roles_to_remove): for role in roles_to_remove: params = {'DBInstanceIdentifier': instance_id, 'RoleArn': role['role_arn'], From 30c973ac5360354afa0efc1fc26c3f5cf998848a Mon Sep 17 00:00:00 2001 From: Joseph Torcasso Date: Tue, 29 Mar 2022 17:35:36 -0400 Subject: [PATCH 06/11] move engine validation to rds_instance and add waiter for promoted read replica --- plugins/module_utils/rds.py | 40 ++++++++++++++++++++++++--------- plugins/module_utils/waiters.py | 25 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/plugins/module_utils/rds.py b/plugins/module_utils/rds.py index fe7fcd4ef02..8aa46a06a43 100644 --- a/plugins/module_utils/rds.py +++ b/plugins/module_utils/rds.py @@ -70,6 +70,8 @@ def get_rds_method_attribute(method_name, module): 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: @@ -135,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' @@ -166,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 @@ -313,6 +306,18 @@ def ensure_tags(client, module, resource_arn, existing_tags, tags, purge_tags): 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 + ''' if target_roles is None: target_roles = [] roles_to_add = [] @@ -340,6 +345,19 @@ def compare_iam_roles(existing_roles, target_roles, purge_roles): 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: AWSModule + instance_id: 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'], diff --git a/plugins/module_utils/waiters.py b/plugins/module_utils/waiters.py index a86ba319f53..087088ba216 100644 --- a/plugins/module_utils/waiters.py +++ b/plugins/module_utils/waiters.py @@ -685,6 +685,25 @@ } ] }, + "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, @@ -1037,6 +1056,12 @@ 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'), From a5efc88242a26f521d2d88f90612564f8900efda Mon Sep 17 00:00:00 2001 From: Joseph Torcasso Date: Tue, 29 Mar 2022 17:38:27 -0400 Subject: [PATCH 07/11] update changelog --- ...ml => 714-module_util_rds-support-iam-roles-add-waiters.yml} | 2 ++ 1 file changed, 2 insertions(+) rename changelogs/fragments/{714-module_util_rds-support-iam-roles.yml => 714-module_util_rds-support-iam-roles-add-waiters.yml} (57%) diff --git a/changelogs/fragments/714-module_util_rds-support-iam-roles.yml b/changelogs/fragments/714-module_util_rds-support-iam-roles-add-waiters.yml similarity index 57% rename from changelogs/fragments/714-module_util_rds-support-iam-roles.yml rename to changelogs/fragments/714-module_util_rds-support-iam-roles-add-waiters.yml index f077b699589..372f8e51d6a 100644 --- a/changelogs/fragments/714-module_util_rds-support-iam-roles.yml +++ b/changelogs/fragments/714-module_util_rds-support-iam-roles-add-waiters.yml @@ -1,2 +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.elbv2 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). From a5b806f0458c2908d6300ec3f47b2555fa69bde8 Mon Sep 17 00:00:00 2001 From: Joseph Torcasso Date: Tue, 29 Mar 2022 20:02:44 -0400 Subject: [PATCH 08/11] remove unit test for error that is handled in rds_instance --- tests/unit/module_utils/test_rds.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/unit/module_utils/test_rds.py b/tests/unit/module_utils/test_rds.py index b485763ee50..e5deca4b8c1 100644 --- a/tests/unit/module_utils/test_rds.py +++ b/tests/unit/module_utils/test_rds.py @@ -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"), From 72f8dfb60d3099e24469bbb7292a24098ec3623e Mon Sep 17 00:00:00 2001 From: Joseph Torcasso <87090265+jatorcasso@users.noreply.github.com> Date: Thu, 31 Mar 2022 16:14:58 -0400 Subject: [PATCH 09/11] Update changelogs/fragments/714-module_util_rds-support-iam-roles-add-waiters.yml Co-authored-by: Jill R <4121322+jillr@users.noreply.github.com> --- .../714-module_util_rds-support-iam-roles-add-waiters.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 372f8e51d6a..611240b23ba 100644 --- 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 @@ -1,4 +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.elbv2 with waiters (https://github.com/ansible-collections/amazon.aws/pull/714). + - 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). From b5a165ddfc6a741acbee0cef6f93102bcd57f318 Mon Sep 17 00:00:00 2001 From: Joseph Torcasso <87090265+jatorcasso@users.noreply.github.com> Date: Thu, 31 Mar 2022 16:15:07 -0400 Subject: [PATCH 10/11] Update plugins/module_utils/rds.py Co-authored-by: Jill R <4121322+jillr@users.noreply.github.com> --- plugins/module_utils/rds.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/module_utils/rds.py b/plugins/module_utils/rds.py index 8aa46a06a43..26eb4e1fe26 100644 --- a/plugins/module_utils/rds.py +++ b/plugins/module_utils/rds.py @@ -350,8 +350,8 @@ def update_iam_roles(client, module, instance_id, roles_to_add, roles_to_remove) Parameters: client: RDS client - module: AWSModule - instance_id: DB's instance ID + 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 From ae4a806d60494a6736e6938742fa62bb6237981a Mon Sep 17 00:00:00 2001 From: Joseph Torcasso Date: Thu, 31 Mar 2022 16:53:49 -0400 Subject: [PATCH 11/11] improve python code quality --- plugins/module_utils/rds.py | 26 +++----------------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/plugins/module_utils/rds.py b/plugins/module_utils/rds.py index 26eb4e1fe26..d656b7e7f12 100644 --- a/plugins/module_utils/rds.py +++ b/plugins/module_utils/rds.py @@ -318,29 +318,9 @@ def compare_iam_roles(existing_roles, target_roles, purge_roles): roles_to_add (list): List of IAM roles to add roles_to_delete (list): List of IAM roles to delete ''' - if target_roles is None: - target_roles = [] - roles_to_add = [] - roles_to_remove = [] - for target_role in target_roles: - found = False - for existing_role in existing_roles: - if target_role['role_arn'] == existing_role['role_arn'] and target_role['feature_name'] == existing_role['feature_name']: - found = True - break - if not found: - roles_to_add.append(target_role) - - if purge_roles: - for existing_role in existing_roles: - found = False - for target_role in target_roles: - if target_role['role_arn'] == existing_role['role_arn'] and target_role['feature_name'] == existing_role['feature_name']: - found = True - break - if not found: - roles_to_remove.append(existing_role) - + 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