From da0027dddd31251c896c1af1f589769e20b332d8 Mon Sep 17 00:00:00 2001 From: Bikouo Aubin <79859644+abikouo@users.noreply.github.com> Date: Mon, 9 Dec 2024 13:06:17 +0100 Subject: [PATCH] rds_cluster - fix issue when updating rds cluster domain (#2401) SUMMARY Closes #2390 Fix issue when trying to update RDS cluster with new domain add unit tests to validate the fix ISSUE TYPE Bugfix Pull Request COMPONENT NAME rds_cluster Reviewed-by: Alina Buzachis --- ...x-issue-occurring-when-updating-domain.yml | 3 + plugins/modules/rds_cluster.py | 36 +- .../unit/plugins/modules/test_rds_cluster.py | 313 ++++++++++++++++++ 3 files changed, 345 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/20241127-rds_cluster-fix-issue-occurring-when-updating-domain.yml create mode 100644 tests/unit/plugins/modules/test_rds_cluster.py diff --git a/changelogs/fragments/20241127-rds_cluster-fix-issue-occurring-when-updating-domain.yml b/changelogs/fragments/20241127-rds_cluster-fix-issue-occurring-when-updating-domain.yml new file mode 100644 index 00000000000..0ecb758dac8 --- /dev/null +++ b/changelogs/fragments/20241127-rds_cluster-fix-issue-occurring-when-updating-domain.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - rds_cluster - Fix issue occurring when updating RDS cluster domain (https://github.com/ansible-collections/amazon.aws/issues/2390). \ No newline at end of file diff --git a/plugins/modules/rds_cluster.py b/plugins/modules/rds_cluster.py index 42bf9be77dd..69f5ffda03c 100644 --- a/plugins/modules/rds_cluster.py +++ b/plugins/modules/rds_cluster.py @@ -1066,14 +1066,18 @@ def changing_cluster_options(modify_params, current_cluster): provided_cloudwatch_logs = set(enable_cloudwatch_logs_export) current_cloudwatch_logs_export = set(current_cluster["EnabledCloudwatchLogsExports"]) - desired_cloudwatch_logs_configuration["EnableLogTypes"] = list( - provided_cloudwatch_logs.difference(current_cloudwatch_logs_export) + desired_cloudwatch_logs_configuration["EnableLogTypes"] = sorted( + list(provided_cloudwatch_logs.difference(current_cloudwatch_logs_export)) ) if module.params["purge_cloudwatch_logs_exports"]: - desired_cloudwatch_logs_configuration["DisableLogTypes"] = list( - current_cloudwatch_logs_export.difference(provided_cloudwatch_logs) + desired_cloudwatch_logs_configuration["DisableLogTypes"] = sorted( + list(current_cloudwatch_logs_export.difference(provided_cloudwatch_logs)) ) - changing_params["CloudwatchLogsExportConfiguration"] = desired_cloudwatch_logs_configuration + if ( + desired_cloudwatch_logs_configuration["EnableLogTypes"] + or desired_cloudwatch_logs_configuration["DisableLogTypes"] + ): + changing_params["CloudwatchLogsExportConfiguration"] = desired_cloudwatch_logs_configuration password = modify_params.pop("MasterUserPassword", None) if password: @@ -1095,10 +1099,10 @@ def changing_cluster_options(modify_params, current_cluster): provided_vpc_sgs = set(vpc_sgs) current_vpc_sgs = set([sg["VpcSecurityGroupId"] for sg in current_cluster["VpcSecurityGroups"]]) if module.params["purge_security_groups"]: - desired_vpc_sgs = vpc_sgs + desired_vpc_sgs = sorted(list(provided_vpc_sgs)) else: if provided_vpc_sgs - current_vpc_sgs: - desired_vpc_sgs = list(provided_vpc_sgs | current_vpc_sgs) + desired_vpc_sgs = sorted(list(provided_vpc_sgs | current_vpc_sgs)) if desired_vpc_sgs: changing_params["VpcSecurityGroupIds"] = desired_vpc_sgs @@ -1113,6 +1117,24 @@ def changing_cluster_options(modify_params, current_cluster): if param == "StorageType": changing_params[param] = modify_params[param] continue + # describe_db_clusters returns the Domain member ship information as follow + # 'DomainMemberships': [ + # { + # 'Domain': 'string', + # 'Status': 'string', + # 'FQDN': 'string', + # 'IAMRoleName': 'string' + # }, + # ] + if param in ("Domain", "DomainIAMRoleName"): + n_domain = modify_params.get("Domain") + n_domain_iam_role_name = modify_params.get("DomainIAMRoleName") + if not any( + n_domain == d.get("Domain") and n_domain_iam_role_name == d.get("IAMRoleName") + for d in current_cluster.get("DomainMemberships", []) + ): + changing_params[param] = modify_params[param] + continue if modify_params[param] != current_cluster[param]: changing_params[param] = modify_params[param] diff --git a/tests/unit/plugins/modules/test_rds_cluster.py b/tests/unit/plugins/modules/test_rds_cluster.py new file mode 100644 index 00000000000..88fcfd6ffc1 --- /dev/null +++ b/tests/unit/plugins/modules/test_rds_cluster.py @@ -0,0 +1,313 @@ +# (c) 2024 Red Hat Inc. +# +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +import copy +from unittest.mock import MagicMock + +import pytest + +from ansible_collections.amazon.aws.plugins.modules import rds_cluster + + +@pytest.fixture +def ansible_module(): + module = MagicMock() + module.check_mode = False + module.params = {} + module.fail_json.side_effect = SystemExit(1) + module.fail_json_aws.side_effect = SystemExit(1) + + return module + + +class TestChangingClusterOptions: + def test_module_state(self, ansible_module): + apply_immediately = MagicMock() + db_cluster_id = MagicMock() + master_user_passwd = MagicMock() + + modify_params = { + "ApplyImmediately": apply_immediately, + "DBClusterIdentifier": db_cluster_id, + "MasterUserPassword": master_user_passwd, + } + current_cluster = { + "DBClusterIdentifier": db_cluster_id, + } + + # 'present' or 'absent' + rds_cluster.module = ansible_module + for state in ("present", "absent"): + rds_cluster.module.params.update({"state": state}) + assert modify_params == rds_cluster.changing_cluster_options(copy.deepcopy(modify_params), current_cluster) + + # 'stopped'/'started' + for state in ("started", "stopped"): + rds_cluster.module.params.update({"state": state}) + # 'mysql' or 'postgres' engine + for engine in ("mysql", "postgres"): + current_cluster.update({"Engine": engine}) + with pytest.raises(SystemExit): + rds_cluster.changing_cluster_options(copy.deepcopy(modify_params), current_cluster) + rds_cluster.module.fail_json.assert_called_once_with(f"Only aurora clusters can use the state {state}") + rds_cluster.module.fail_json.reset_mock() + # 'aurora' engine + current_cluster.update({"Engine": "aurora"}) + assert {"DBClusterIdentifier": db_cluster_id} == rds_cluster.changing_cluster_options( + copy.deepcopy(modify_params), current_cluster + ) + + @pytest.mark.parametrize( + "params,expected", + [ + ({"StorageType": "10B"}, {"StorageType": "10B"}), + ({"StorageType": "10B", "DeletionProtection": True}, {"StorageType": "10B", "DeletionProtection": True}), + ({"DeletionProtection": True}, {"DeletionProtection": True}), + ({"DeletionProtection": False}, {}), + ({"Domain": "domain-2"}, {"Domain": "domain-2"}), + ({"Domain": "domain-1", "DomainIAMRoleName": "ansible-test-units-001"}, {}), + ( + {"Domain": "domain-2", "DomainIAMRoleName": "ansible-test-units-001"}, + {"Domain": "domain-2", "DomainIAMRoleName": "ansible-test-units-001"}, + ), + ( + {"Domain": "domain-2", "DomainIAMRoleName": "ansible-test-units-002"}, + {"Domain": "domain-2", "DomainIAMRoleName": "ansible-test-units-002"}, + ), + ], + ) + def test_modify_params(self, ansible_module, params, expected): + apply_immediately = MagicMock() + db_cluster_id = MagicMock() + + modify_params = { + "ApplyImmediately": apply_immediately, + "DBClusterIdentifier": db_cluster_id, + } + if expected: + expected.update(modify_params) + modify_params.update(params) + + current_cluster = { + "DBClusterIdentifier": db_cluster_id, + "DeletionProtection": False, + "DomainMemberships": [ + { + "Domain": "domain-1", + "Status": "joined", + "FQDN": "testing.ansible.units", + "IAMRoleName": "ansible-test-units-001", + }, + ], + } + + # state is set to 'present' + ansible_module.params.update({"state": "present"}) + rds_cluster.module = ansible_module + assert expected == rds_cluster.changing_cluster_options(modify_params, current_cluster) + + @pytest.mark.parametrize( + "modify_vpc_security_group_ids, current_vpc_security_group_ids, expected_vpc_security_group_ids", + [ + (["id-1", "id-1", "id-2"], ["id-3"], ["id-1", "id-2"]), + (["id-1", "id-2"], ["id-1", "id-2"], ["id-1", "id-2"]), + ], + ) + def test_vpc_security_group_ids_purge_true( + self, + ansible_module, + modify_vpc_security_group_ids, + current_vpc_security_group_ids, + expected_vpc_security_group_ids, + ): + db_cluster_id = MagicMock() + apply_immediately = MagicMock() + modify_params = { + "DBClusterIdentifier": db_cluster_id, + "ApplyImmediately": apply_immediately, + } + expected = {} + if expected_vpc_security_group_ids: + expected.update(modify_params) + expected["VpcSecurityGroupIds"] = expected_vpc_security_group_ids + modify_params.update({"VpcSecurityGroupIds": modify_vpc_security_group_ids}) + + current_cluster = {"VpcSecurityGroups": [{"VpcSecurityGroupId": x} for x in current_vpc_security_group_ids]} + + # state is set to 'present' + ansible_module.params.update({"state": "present", "purge_security_groups": True}) + rds_cluster.module = ansible_module + assert expected == rds_cluster.changing_cluster_options(modify_params, current_cluster) + + @pytest.mark.parametrize( + "modify_vpc_security_group_ids, current_vpc_security_group_ids, expected_vpc_security_group_ids", + [ + (["id-1", "id-1", "id-2"], ["id-3"], ["id-1", "id-2", "id-3"]), + (["id-1", "id-2"], ["id-2", "id-1"], []), + (["id-1", "id-2"], ["id-2", "id-1", "id-3"], []), + (["id-1", "id-1", "id-2"], [], ["id-1", "id-2"]), + ], + ) + def test_vpc_security_group_ids_purge_false( + self, + ansible_module, + modify_vpc_security_group_ids, + current_vpc_security_group_ids, + expected_vpc_security_group_ids, + ): + db_cluster_id = MagicMock() + apply_immediately = MagicMock() + modify_params = { + "DBClusterIdentifier": db_cluster_id, + "ApplyImmediately": apply_immediately, + } + expected = {} + if expected_vpc_security_group_ids: + expected.update(modify_params) + expected["VpcSecurityGroupIds"] = expected_vpc_security_group_ids + modify_params.update({"VpcSecurityGroupIds": modify_vpc_security_group_ids}) + + current_cluster = {"VpcSecurityGroups": [{"VpcSecurityGroupId": x} for x in current_vpc_security_group_ids]} + + # state is set to 'present' + ansible_module.params.update({"state": "present", "purge_security_groups": False}) + rds_cluster.module = ansible_module + assert expected == rds_cluster.changing_cluster_options(modify_params, current_cluster) + + @pytest.mark.parametrize( + "purge_cloudwatch_logs_exports, modify_enable_cloudwatch_logs_exports, current_enabled_cloudwatch_logs_exports, enable_log_types, disable_log_types", + [ + (True, ["log-3", "log-2"], ["log-1"], ["log-2", "log-3"], ["log-1"]), + (False, ["log-3", "log-2"], ["log-1"], ["log-2", "log-3"], []), + (True, ["log-1", "log-2"], ["log-1", "log-2", "log-3"], [], ["log-3"]), + (False, ["log-1", "log-2"], ["log-1", "log-2", "log-3"], [], []), + ], + ) + def test_enable_cloudwatch_logs_export( + self, + ansible_module, + purge_cloudwatch_logs_exports, + modify_enable_cloudwatch_logs_exports, + current_enabled_cloudwatch_logs_exports, + enable_log_types, + disable_log_types, + ): + db_cluster_id = MagicMock() + apply_immediately = MagicMock() + modify_params = { + "DBClusterIdentifier": db_cluster_id, + "ApplyImmediately": apply_immediately, + "EnableCloudwatchLogsExports": modify_enable_cloudwatch_logs_exports, + } + expected = None + if enable_log_types or disable_log_types: + expected = {"EnableLogTypes": enable_log_types, "DisableLogTypes": disable_log_types} + + current_cluster = {"EnabledCloudwatchLogsExports": current_enabled_cloudwatch_logs_exports} + + # state is set to 'present' + ansible_module.params.update( + {"state": "present", "purge_cloudwatch_logs_exports": purge_cloudwatch_logs_exports} + ) + rds_cluster.module = ansible_module + assert expected == rds_cluster.changing_cluster_options(modify_params, current_cluster).get( + "CloudwatchLogsExportConfiguration" + ) + + @pytest.mark.parametrize( + "new_cluster_id, current_cluster_id, expected_change", + [ + ("", "cluster-id-1", False), + (None, "cluster-id-1", False), + ("cluster-id-1", "cluster-id-1", False), + ("cluster-id-2", "cluster-id-1", True), + ], + ) + def test_modify_cluster_id(self, ansible_module, new_cluster_id, current_cluster_id, expected_change): + db_cluster_id = MagicMock() + apply_immediately = MagicMock() + modify_params = { + "DBClusterIdentifier": db_cluster_id, + "NewDBClusterIdentifier": new_cluster_id, + "ApplyImmediately": apply_immediately, + } + expected = {} + if expected_change: + expected.update(modify_params) + + current_cluster = {"DBClusterIdentifier": current_cluster_id} + + # state is set to 'present' + ansible_module.params.update({"state": "present"}) + rds_cluster.module = ansible_module + assert expected == rds_cluster.changing_cluster_options(modify_params, current_cluster) + + @pytest.mark.parametrize( + "new_option_group_name, cluster_option_group_memberships, expected_change", + [ + ("", [], False), + (None, [], False), + ("opt-3", ["opt-1", "opt-2", "opt-3"], False), + ("opt-3", ["opt-1", "opt-2"], True), + ], + ) + def test_option_group_name( + self, ansible_module, new_option_group_name, cluster_option_group_memberships, expected_change + ): + db_cluster_id = MagicMock() + apply_immediately = MagicMock() + modify_params = { + "DBClusterIdentifier": db_cluster_id, + "OptionGroupName": new_option_group_name, + "ApplyImmediately": apply_immediately, + } + expected = {} + if expected_change: + expected.update(modify_params) + + current_cluster = { + "DBClusterOptionGroupMemberships": [ + {"DBClusterOptionGroupName": x} for x in cluster_option_group_memberships + ] + } + + # state is set to 'present' + ansible_module.params.update({"state": "present"}) + rds_cluster.module = ansible_module + assert expected == rds_cluster.changing_cluster_options(modify_params, current_cluster) + + @pytest.mark.parametrize( + "new_db_cluster_parameter_group_name, current_db_cluster_parameter_group_name, expected_change", + [ + ("", "param-group-0", False), + (None, "param-group-0", False), + ("param-group-0", "param-group-0", False), + ("param-group-1", "param-group-0", True), + ], + ) + def test_db_cluster_parameter_group_name( + self, + ansible_module, + new_db_cluster_parameter_group_name, + current_db_cluster_parameter_group_name, + expected_change, + ): + db_cluster_id = MagicMock() + apply_immediately = MagicMock() + modify_params = { + "DBClusterIdentifier": db_cluster_id, + "DBClusterParameterGroupName": new_db_cluster_parameter_group_name, + "ApplyImmediately": apply_immediately, + } + expected = {} + if expected_change: + expected.update(modify_params) + + current_cluster = {"DBClusterParameterGroup": current_db_cluster_parameter_group_name} + + # state is set to 'present' + ansible_module.params.update({"state": "present"}) + rds_cluster.module = ansible_module + assert expected == rds_cluster.changing_cluster_options(modify_params, current_cluster)