From c8de1c025a71e4233c54d797b2579b412a8e4a9e Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Wed, 28 Aug 2024 08:39:37 +0000 Subject: [PATCH] s3_bucket - fix transfer acceleration issue with AWS GovCloud (#2194) (#2202) This is a backport of PR #2194 as merged into main (7d6a5ef). SUMMARY Closes #2180 Transfer acceleration is not available for AWS GovCloud region, this PR performs the following changes GetBucketAccelerateConfiguration operation is performed only if the user has provided accelerate_enabled option When GetBucketAccelerateConfiguration raises an UnsupportedArgument botocore exception, the module will display a warning message accelerate_enabled is set in the output only if it was provided as input ISSUE TYPE Bugfix Pull Request COMPONENT NAME s3_bucket Reviewed-by: Mark Chappell --- ...0240715-s3_bucket-transfer-accelerate.yaml | 3 + plugins/modules/s3_bucket.py | 9 +- .../plugins/modules/s3_bucket/__init__.py | 0 .../test_handle_bucket_accelerate.py | 90 +++++++++++++++++++ 4 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/20240715-s3_bucket-transfer-accelerate.yaml create mode 100644 tests/unit/plugins/modules/s3_bucket/__init__.py create mode 100644 tests/unit/plugins/modules/s3_bucket/test_handle_bucket_accelerate.py diff --git a/changelogs/fragments/20240715-s3_bucket-transfer-accelerate.yaml b/changelogs/fragments/20240715-s3_bucket-transfer-accelerate.yaml new file mode 100644 index 00000000000..1d31b7bd013 --- /dev/null +++ b/changelogs/fragments/20240715-s3_bucket-transfer-accelerate.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - s3_bucket - catch ``UnsupportedArgument`` when calling API ``GetBucketAccelerationConfig`` on region where it is not supported (https://github.com/ansible-collections/amazon.aws/issues/2180). \ No newline at end of file diff --git a/plugins/modules/s3_bucket.py b/plugins/modules/s3_bucket.py index d2ed77f2a1d..fd954de312b 100644 --- a/plugins/modules/s3_bucket.py +++ b/plugins/modules/s3_bucket.py @@ -155,7 +155,7 @@ description: - Whether the bucket name should be validated to conform to AWS S3 naming rules. - On by default, this may be disabled for S3 backends that do not enforce these rules. - - See https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html + - See U(https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html). type: bool version_added: 3.1.0 default: true @@ -169,6 +169,8 @@ accelerate_enabled: description: - Enables Amazon S3 Transfer Acceleration, sent data will be routed to Amazon S3 over an optimized network path. + - Transfer Acceleration is not available in AWS GovCloud (US). + - See U(https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-s3.html#govcloud-S3-diffs). type: bool version_added: 8.1.0 object_lock_default_retention: @@ -970,6 +972,11 @@ def handle_bucket_accelerate(s3_client, module: AnsibleAWSModule, name: str) -> except is_boto3_error_code(["NotImplemented", "XNotImplemented"]) as e: if accelerate_enabled is not None: module.fail_json_aws(e, msg="Fetching bucket transfer acceleration state is not supported") + except is_boto3_error_code("UnsupportedArgument") as e: # pylint: disable=duplicate-except + # -- Transfer Acceleration is not available in AWS GovCloud (US). + # -- https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-s3.html#govcloud-S3-diffs + module.warn("Tranfer acceleration is not available in S3 bucket region.") + accelerate_enabled_result = False except is_boto3_error_code("AccessDenied") as e: # pylint: disable=duplicate-except if accelerate_enabled is not None: module.fail_json_aws(e, msg="Permission denied fetching transfer acceleration for bucket") diff --git a/tests/unit/plugins/modules/s3_bucket/__init__.py b/tests/unit/plugins/modules/s3_bucket/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/unit/plugins/modules/s3_bucket/test_handle_bucket_accelerate.py b/tests/unit/plugins/modules/s3_bucket/test_handle_bucket_accelerate.py new file mode 100644 index 00000000000..c02ec806c90 --- /dev/null +++ b/tests/unit/plugins/modules/s3_bucket/test_handle_bucket_accelerate.py @@ -0,0 +1,90 @@ +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from unittest.mock import MagicMock +from unittest.mock import patch +from unittest.mock import sentinel + +import botocore +import pytest + +from ansible_collections.amazon.aws.plugins.modules.s3_bucket import handle_bucket_accelerate + +module_name = "ansible_collections.amazon.aws.plugins.modules.s3_bucket" + + +def a_botocore_exception(message): + return botocore.exceptions.ClientError({"Error": {"Code": message}}, sentinel.BOTOCORE_ACTION) + + +@pytest.fixture +def ansible_module(): + mock = MagicMock() + mock.params = {"accelerate_enabled": sentinel.ACCELERATE_ENABLED} + mock.fail_json_aws.side_effect = SystemExit(1) + return mock + + +@pytest.mark.parametrize( + "code,message", + [ + ("NotImplemented", "Fetching bucket transfer acceleration state is not supported"), + ("XNotImplemented", "Fetching bucket transfer acceleration state is not supported"), + ("AccessDenied", "Permission denied fetching transfer acceleration for bucket"), + (sentinel.BOTO_CLIENT_ERROR, "Failed to fetch bucket transfer acceleration state"), + ], +) +@patch(module_name + ".get_bucket_accelerate_status") +def test_failure(m_get_bucket_accelerate_status, ansible_module, code, message): + bucket_name = sentinel.BUCKET_NAME + client = MagicMock() + exc = a_botocore_exception(code) + m_get_bucket_accelerate_status.side_effect = exc + with pytest.raises(SystemExit): + handle_bucket_accelerate(client, ansible_module, bucket_name) + ansible_module.fail_json_aws.assert_called_once_with(exc, msg=message) + + +@patch(module_name + ".get_bucket_accelerate_status") +def test_unsupported(m_get_bucket_accelerate_status, ansible_module): + bucket_name = sentinel.BUCKET_NAME + client = MagicMock() + m_get_bucket_accelerate_status.side_effect = a_botocore_exception("UnsupportedArgument") + changed, result = handle_bucket_accelerate(client, ansible_module, bucket_name) + assert changed is False + assert result is False + ansible_module.warn.assert_called_once() + + +@pytest.mark.parametrize("accelerate_enabled", [True, False]) +@patch(module_name + ".delete_bucket_accelerate_configuration") +@patch(module_name + ".get_bucket_accelerate_status") +def test_delete( + m_get_bucket_accelerate_status, m_delete_bucket_accelerate_configuration, ansible_module, accelerate_enabled +): + bucket_name = sentinel.BUCKET_NAME + client = MagicMock() + ansible_module.params.update({"accelerate_enabled": accelerate_enabled}) + m_get_bucket_accelerate_status.return_value = True + if not accelerate_enabled: + assert (True, False) == handle_bucket_accelerate(client, ansible_module, bucket_name) + m_delete_bucket_accelerate_configuration.assert_called_once_with(client, bucket_name) + else: + assert (False, True) == handle_bucket_accelerate(client, ansible_module, bucket_name) + m_delete_bucket_accelerate_configuration.assert_not_called() + + +@pytest.mark.parametrize("accelerate_enabled", [True, False]) +@patch(module_name + ".put_bucket_accelerate_configuration") +@patch(module_name + ".get_bucket_accelerate_status") +def test_put(m_get_bucket_accelerate_status, m_put_bucket_accelerate_configuration, ansible_module, accelerate_enabled): + bucket_name = sentinel.BUCKET_NAME + client = MagicMock() + ansible_module.params.update({"accelerate_enabled": accelerate_enabled}) + m_get_bucket_accelerate_status.return_value = False + if accelerate_enabled: + assert (True, True) == handle_bucket_accelerate(client, ansible_module, bucket_name) + m_put_bucket_accelerate_configuration.assert_called_once_with(client, bucket_name) + else: + assert (False, False) == handle_bucket_accelerate(client, ansible_module, bucket_name) + m_put_bucket_accelerate_configuration.assert_not_called()