From f3b8b46469775e7a402fb1e87fd8fad8125f48e3 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 16 May 2023 11:19:42 +0200 Subject: [PATCH] s3_object - fix regression related to leading / in object key names --- .../1548-s3_object-leading-slash.yml | 4 ++ plugins/modules/s3_object.py | 37 +++++++++++++------ tests/unit/plugins/modules/test_s3_object.py | 16 ++++++-- 3 files changed, 42 insertions(+), 15 deletions(-) create mode 100644 changelogs/fragments/1548-s3_object-leading-slash.yml diff --git a/changelogs/fragments/1548-s3_object-leading-slash.yml b/changelogs/fragments/1548-s3_object-leading-slash.yml new file mode 100644 index 00000000000..954abe100eb --- /dev/null +++ b/changelogs/fragments/1548-s3_object-leading-slash.yml @@ -0,0 +1,4 @@ +bugfixes: +- s3_object - fixes regression related to objects with a leading ``/`` (https://github.com/ansible-collections/amazon.aws/issues/1548). +deprecated_features: +- s3_object - support for passing object keys with a leading ``/`` has been deprecated and will be removed in a release after 2025-12-01 (). diff --git a/plugins/modules/s3_object.py b/plugins/modules/s3_object.py index 6570bccd8c2..b6e5b966793 100644 --- a/plugins/modules/s3_object.py +++ b/plugins/modules/s3_object.py @@ -89,8 +89,12 @@ type: str object: description: - - Keyname of the object inside the bucket. + - Key name of the object inside the bucket. - Can be used to create "virtual directories", see examples. + - Object key names should not include the leading C(/), see + U(https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html) for more + information. Support for passing the leading C(/) has been deprecated and will be removed + in a release after 2025-12-01. type: str sig_v4: description: @@ -399,13 +403,13 @@ - prefix1/key2 """ +import base64 +import copy +import io import mimetypes import os -import io -from ssl import SSLError -import base64 import time - +from ssl import SSLError try: import botocore @@ -1325,9 +1329,9 @@ def s3_object_do_copy(module, connection, connection_v4, s3_vars): ) -def populate_facts(module, **variable_dict): - for k, v in module.params.items(): - variable_dict[k] = v +def populate_params(module): + # Copy the parameters dict, we shouldn't be directly modifying it. + variable_dict = copy.deepcopy(module.params) if variable_dict["validate_bucket_name"]: validate_bucket_name(variable_dict["bucket"]) @@ -1347,8 +1351,19 @@ def populate_facts(module, **variable_dict): variable_dict["overwrite"] = "never" # Bucket deletion does not require obj. Prevents ambiguity with delobj. - if variable_dict["object"] and variable_dict.get("mode") == "delete": - module.fail_json(msg="Parameter obj cannot be used with mode=delete") + if variable_dict["object"]: + if variable_dict.get("mode") == "delete": + module.fail_json(msg="Parameter object cannot be used with mode=delete") + obj = variable_dict["object"] + # If the object starts with / remove the leading character + if obj.startswith('/'): + obj = obj[1:] + variable_dict["object"] = obj + module.deprecate( + "Support for passing object key names with a leading '/' has been deprecated.", + date="2025-12-01", + collection_name="amazon.aws", + ) variable_dict["validate"] = not variable_dict["ignore_nonexistent_bucket"] variable_dict["acl_disabled"] = False @@ -1483,7 +1498,7 @@ def main(): except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Failed to connect to AWS") - s3_object_params = populate_facts(module, **module.params) + s3_object_params = populate_params(module, **module.params) s3_object_params.update(validate_bucket(module, s3, s3_object_params)) func_mapping = { diff --git a/tests/unit/plugins/modules/test_s3_object.py b/tests/unit/plugins/modules/test_s3_object.py index 630efff4575..c8d3fc4fcf3 100644 --- a/tests/unit/plugins/modules/test_s3_object.py +++ b/tests/unit/plugins/modules/test_s3_object.py @@ -95,7 +95,7 @@ def test_s3_object_do_list_success(m_paginated_list, m_list_keys): @patch(utils + ".get_aws_connection_info") -def test_populate_facts(m_get_aws_connection_info): +def test_populate_params(m_get_aws_connection_info): module = MagicMock() m_get_aws_connection_info.return_value = ( "us-east-1", @@ -143,10 +143,18 @@ def test_populate_facts(m_get_aws_connection_info): "validate_certs": True, "version": None, } - result = s3_object.populate_facts(module) + result = s3_object.populate_params(module) for k, v in module.params.items(): assert result[k] == v + module.params.update({"object": "example.txt", "mode": "get"}) + result = s3_object.populate_params(module) + assert result["object"] == "example.txt" + + module.params.update({"object": "/example.txt", "mode": "get"}) + result = s3_object.populate_params(module) + assert result["object"] == "example.txt" + module.params.update({"object": "example.txt", "mode": "delete"}) - result = s3_object.populate_facts(module) - module.fail_json.assert_called_with(msg="Parameter obj cannot be used with mode=delete") + result = s3_object.populate_params(module) + module.fail_json.assert_called_with(msg="Parameter object cannot be used with mode=delete")